Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions __tests__/index.test.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,49 @@ describe('multiple variables', () => {
expect(container.querySelector('select')).not.toBeInTheDocument();
});

it('should render from the top level if not set', () => {
const { container } = render(
<Variable
{...props}
user={{
topLevelProperty: 'this is coming straight from the top',
keys: [{ name: 'project1', apiKey: '123' }],
}}
variable={'topLevelProperty'}
/>
);

expect(container).toHaveTextContent('this is coming straight from the top');

// Should not show the selected dropdown when clicked
fireEvent.click(container.querySelector('.variable-underline'));
expect(container.querySelector('select')).not.toBeInTheDocument();
});

it('should render the default if not set', () => {
const { container } = render(
<Variable
{...props}
defaults={[{ name: 'testDefault', default: 'this is a default value' }]}
variable={'testDefault'}
/>
);

expect(container).toHaveTextContent('this is a default value');

// Should not show the selected dropdown when clicked
fireEvent.click(container.querySelector('.variable-underline'));
expect(container.querySelector('select')).not.toBeInTheDocument();
});

it('should not show a dropdown if there is only one option', () => {
const { container } = render(<Variable {...props} user={{ keys: [{ name: 'project1', apiKey: '123' }] }} />);

// Should not show the selected dropdown when clicked
fireEvent.click(container.querySelector('.variable-underline'));
expect(container.querySelector('select')).not.toBeInTheDocument();
});

it.todo('should render auth dropdown if default and oauth enabled');
});

Expand Down
35 changes: 29 additions & 6 deletions index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,18 +27,42 @@ class Variable extends React.Component {
return this.props.variable.toUpperCase();
}

getSelectedValue(selected) {
const { user } = this.props;
let selectedValue = {};
if (Array.isArray(user.keys)) {
selectedValue = selected ? user.keys.find(key => key.name === selected) : user.keys[0];
}
return selectedValue;
}

// Determining whether to show a dropdown or not onClick
// based upon whether the currently displayed value has come
// from one of the nested user.keys[] or not.
// Additionally do not show the dropdown if there is only a
// single key, since there's nothing to change it to.
shouldShowVarDropdown(selected) {
const { user, variable } = this.props;

return !!this.getSelectedValue(selected)[variable] && Array.isArray(user.keys) && user.keys.length > 1;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should it only count keys with values? Is that possible?

Suggested change
return !!this.getSelectedValue(selected)[variable] && Array.isArray(user.keys) && user.keys.length > 1;
return !!this.getSelectedValue(selected)[variable] && Array.isArray(user.keys) && user.keys.filter(key => key.name === variable).length > 1;

Copy link
Member Author

@domharrington domharrington Jun 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're already checking if this particular variable has a value assigned in the first part of this statement:

!!this.getSelectedValue(selected)[variable]

Is that sufficient for the use case you're referring to? If there isn't a user.keys[variable] then it'll already have exited early from that statement because we already know not to show the var dropdown. So at that point, checking the length should suffice? Unless i'm misunderstanding.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha gotcha, misread that.

}

// Return value in this order
// - user value
// - selected user keys value (or first user keys)
// - top level user value
// - default value
// - uppercase key
getValue() {
getValue(selected) {
const { variable } = this.props;
const value = this.props.user[variable] || this.getDefault();
const selectedValue = this.getSelectedValue(selected);

const value = selectedValue[variable] || this.props.user[variable] || this.getDefault();

return typeof value === 'object' ? JSON.stringify(value) : value;
}

toggleVarDropdown() {
if (!this.shouldShowVarDropdown()) return;
this.setState(prevState => ({ showDropdown: !prevState.showDropdown }));
}

Expand Down Expand Up @@ -81,16 +105,15 @@ class Variable extends React.Component {
}

render() {
const { variable, user, selected } = this.props;
const { user, selected } = this.props;

if (Array.isArray(user.keys)) {
const selectedValue = selected ? user.keys.find(key => key.name === selected) : user.keys[0];
return (
<span>
{!this.state.showDropdown && (
// eslint-disable-next-line jsx-a11y/click-events-have-key-events, jsx-a11y/no-static-element-interactions
<span className="variable-underline" onClick={this.toggleVarDropdown}>
{selectedValue[variable]}
{this.getValue(selected)}
</span>
)}
{this.state.showDropdown && this.renderVarDropdown()}
Expand Down