Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UI bug fix: Kubernetes Role filter replace with explicit input filter #27178

Merged
merged 15 commits into from
Jun 3, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
make div and not form due to testing things
  • Loading branch information
Monkeychip committed May 22, 2024
commit 8356a8123e81a81561fbf9055ad6ca09b46482c1
13 changes: 10 additions & 3 deletions ui/lib/core/addon/components/filter-input-explicit.hbs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
SPDX-License-Identifier: BUSL-1.1
~}}

<form {{on "submit" @handleSearch}}>
<div>
<Hds::SegmentedGroup as |S|>
<S.TextInput
@value={{@query}}
Expand All @@ -14,6 +14,13 @@
{{on "keydown" @handleKeyDown}}
data-test-filter-input-explicit
/>
<S.Button @color="secondary" @text="Search" @icon="search" type="submit" data-test-filter-input-explicit-search />
<S.Button
@color="secondary"
@text="Search"
@icon="search"
type="submit"
{{on "click" @handleSearch}}
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason we aren't wrapping these in a <form> element so that clicking this button submits (and then we also get a free submit when the user presses the "Enter" button)? I think that would be a nice ergonomic update to this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The tests 😵‍💫 I originally had it wrapped as a form, but couldn't call the action correctly from the tests. There was an error about it being in a form element. I wish I would have saved that specific error. Let me see if I can resurface it. The enter functionality would be very nice!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@hashishaw This was the error, any thoughts? Only on the test, not on the UI.
image

Copy link
Contributor

Choose a reason for hiding this comment

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

oh, we just need to do evt.preventDefault in the triggered action so that it doesn't "send" the form data to the URL

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it! Found an example in how we do it for sinon spies in toggle-test (thank you—you did that block of code).

data-test-filter-input-explicit-search
/>
</Hds::SegmentedGroup>
</form>
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ module('Integration | Component | filter-input-explicit', function (hooks) {
test('it should call handleSearch on submit', async function (assert) {
assert.expect(1);

this.handleSubmit = () => {
this.handleSearch = () => {
assert.ok(true, 'handleSearch was called');
};

Expand Down
Loading