-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Create migration guidance for filter() function
#5046
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
Merged
Merged
Changes from all commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
0851ec0
Add filter documentation to new sass functions and mixins section
lgriffee 836558d
Include shorthand function names
lgriffee 2204a86
Remove token replacement section
lgriffee b2384ac
Add quotes around all parameters
lgriffee df13cd4
Add more context to replacing functions section
lgriffee b3f895c
Add in token replacement values with note
lgriffee b652e04
Use footnote Markdown
lgriffee 1ca31ea
Make language reflective of mixins as well
lgriffee 14bba9e
Merge branch 'v9.0.0-major' of https://github.com/Shopify/polaris-rea…
lgriffee b2a10ae
Get rid of duplicate copy
lgriffee 6d3f67d
Add backticks around all values in table
lgriffee 47d5d44
Fix typo
lgriffee 22edcb1
Remove use of var in tokens
lgriffee ea7ccc8
Merge branch 'v9.0.0-major' into 5008-migration-guide-filter-function
lgriffee f792903
Merge branch 'v9.0.0-major' into 5008-migration-guide-filter-function
lgriffee 08df029
Remove color tokens from table
lgriffee b738547
Merge branch 'v9.0.0-major' into 5008-migration-guide-filter-function
lgriffee File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or 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
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.
Uh oh!
There was an error while loading. Please reload this page.
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.
Thoughts on breaking this
filter()section in two parts?<summary>) Here are all the values we've replaced with tokens/values NOT css filters<summary>) Here is an exhaustive list of all legacy css filters you can use for backward compatibilityUh oh!
There was an error while loading. Please reload this page.
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.
Good callout, I was also wondering what's the clearest and most helpful documentation for folks in this case. I ended up combining tokens and values into one table for the
easing()guidance so I was trying to be consistent with that approach here.But perhaps this is just such a giant table and the tokens aren't 1:1 replacements so it makes sense to break them out into two here. I'm totally down with doing that if it feels more clear.
If we do break them into two tables for
filter(), I do want to just quickly consider if we should do the same foreasing()to be consistent? Some of the easing tokens aren't 1:1 replacements for the previous function values. However, unlikefilter()they are improvements oneasing()values versus values meant for another property, which makes me lean towards thinkingeasing()values and tokens are fine in the same table together vs. two tables even if we break thefilter()table up.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.
Personally, I think the guidance should help folks get their apps back to a functional state as quickly as possible. We were able to take some time throughout the process to update and test new ideas, but others may not have that luxury. This is just my 2 cents, so I wonder what other folks think. @alex-page @aveline Thoughts?
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.
Spoke with @alex-page about this and we decided to remove the color token replacement values from the table and just have a general note/disclaimer at the top that our team replaced some instances of the function with color tokens (and that folks should use caution if they do that too).