-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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: Update custom message list filters #26653
Conversation
CI Results: |
Build Results: |
@action | ||
onFilterInputChange(pageFilter) { | ||
this.transitionToMessagesWithParams({ pageFilter }); | ||
@task |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nicely handled!
@@ -126,10 +96,10 @@ module('Acceptance | Enterprise | config-ui/message', function (hooks) { | |||
test('it should show an empty state when no messages are created', async function (assert) { | |||
assert.expect(4); | |||
await click(CUSTOM_MESSAGES.navLink); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice clean up work
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great! You have a checkbox to run enterprise tests, a reminder to select that once you confirmed they've passed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! Thanks for adding the test coverage, just a few comments/questions. Nothing blocking
data-test-filter-by-message-type | ||
/> | ||
<form {{on "submit" (perform this.handleSearch)}} aria-label="Filter custom message list"> | ||
<Hds::SegmentedGroup as |S|> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👏
@task | ||
*handleSearch(evt) { | ||
evt.preventDefault(); | ||
const formData = new FormData(evt.target); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 today I learned about the FormData
class. Is this why you don't have to manage and track the value on the form inputs in the hbs file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's right! As long as we pass a name
to the input, we can access the values by name from the form submit event. When we don't need to show other things based on form values or do validation on keyup, this is a simple way to handle form submits
As part of managing the Ember 5 deprecations, the custom messages filters were causing errors in tests due to the DOM tearing down before the focus filter is able to finish running. Updating filters on type is a pattern we're moving away from, so to resolve the test failures we are replacing this filter bar with an explicit filter button.