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

Update search input component to glimmer #814

Merged
merged 4 commits into from
Nov 18, 2022
Merged

Update search input component to glimmer #814

merged 4 commits into from
Nov 18, 2022

Conversation

geneukum
Copy link
Contributor

Let's update the SearchInput component to extend glimmer/component instead of ember/component. Glimmer components offer a bunch of various advantages over classic components and are the direction that ember is
heading in general.

While we're here, let's prefer single quotes to double quotes throughout our template file, cleanup some test deprecations, and re-enable all of the ESLint rules for which this component was the only violator.

This should resolve the app/components/search-input.js section of #805.

Let's update the SearchInput component to extend glimmer/component
instead of ember/component. Glimmer components offer a bunch of various
advantages over classic components and are the direction that ember is
heading in general.

While we're here, let's also re-enable the eslint rules for which the
SearchInput component had the only violations.
Ember's this.render method and @ember/test-helpers's render method are
equivalent, but using @ember/test-helpers' render method is the
recommended approach. As such, let's go ahead and switch this over.

While we're here, let's re-enable this eslint rule as we no longer have
any violations.
Let's prefer angle bracket syntax to curly syntax when invoking the
SearchInput component in our integration tests.
@chriskrycho chriskrycho merged commit 6283639 into ember-learn:master Nov 18, 2022
@geneukum geneukum deleted the update-search-input-component-to-glimmer branch November 19, 2022 00:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants