-
Notifications
You must be signed in to change notification settings - Fork 5
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
replace IconComponent with icon partials #874
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
oshi97
changed the title
Dev/icon partial
Update IconComponent to use iconPartial and bakedInIcon
Jul 1, 2021
cea2aj
reviewed
Jul 1, 2021
cards/document-standard/template.hbs
Outdated
"iconName": "{{iconName}}" | ||
}'></div> | ||
{{> icons/iconPartial | ||
iconName=iconName |
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.
do you know why an extra space is showing up on these diffs? It might not be an issue
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.
not sure I see the extra spaces
cea2aj
reviewed
Jul 1, 2021
tmeyer2115
reviewed
Jul 2, 2021
oshi97
changed the title
Update IconComponent to use iconPartial and bakedInIcon
replace IconComponent with icon partials
Jul 6, 2021
tmeyer2115
approved these changes
Jul 6, 2021
oshi97
added a commit
that referenced
this pull request
Jul 7, 2021
cea2aj
added a commit
that referenced
this pull request
Jul 27, 2021
Add back the HitchhikerCTA divs and classes that were removed in #874 J=SLAP-1473 TEST=visual Inspect the DOM and see the div and class present for the standard card. Inspect the updated Percy snapshots and see some of the icons change size slightly due to the CSS targeting the div. Also see the icon appear for the standard card which was previously missing.
cea2aj
added a commit
that referenced
this pull request
Jul 27, 2021
Add back the HitchhikerCTA divs and classes that were removed in #874 J=SLAP-1473 TEST=visual Inspect the DOM and see the div and class present for the standard card. Inspect the updated Percy snapshots and see some of the icons change size slightly due to the CSS targeting the div. Also see the icon appear for the standard card which was previously missing.
Merged
cea2aj
added a commit
that referenced
this pull request
Aug 24, 2021
### Features - Support for new languages including Chinese (Traditional), Chinese (Simplified), Russian, Polish, Portuguese, Dutch, Arabic, Korean, Swedish, and Hindi (#918) (#900) - A loading indicator can now be enabled on the search bar (#875) - Voice search support (#894) - Analytics, session tracking, and query source can now be toggled through Runtime Config (#869) (#879) - Added validation to ensure that parts of global_config and page configs are valid (#888) - Font preloads are now located in the layouts/preload-fonts.hbs file (#893) - Global config now includes "sessionTrackingEnabled" and "analyticsEventsEnabled" (#898) - Added a partial for additional Answers configuration (#903) - Map pins now support custom label colors (#922) ### Enhancements - The full-page-map "Search This Area" functionality is better optimized when a user quickly zooms in or out of the map (#861) - Improved build time during development mode (#865) - Icons were updated from components to Handlebars partials to improve performance (#874) ### Bugfixes - When upgrading the theme version, the global config will no longer contain duplicate options which are commented out (#916) - When upgrading the theme, the tests folders are now removed which improves build performance (#913)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR updates all IconComponent usages to use the icon partials in the SDK
It also points the test-site to the feature/develop-i18n branch of the SDK,
which is the same commit that develop is currently at, but includes i18n assets
so our spanish snapshots work
The first group of commits do a find and replace for identical (including whitespace like indentation)
usages of IconComponent.
J=SLAP-1297
TEST=manual
test that custom icons can still be set for card CTAs, section titles, and the searchbar
see that built in icons still show up