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.
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
#26126: Tag menu item and Tag picker (1st PR) #26954
#26126: Tag menu item and Tag picker (1st PR) #26954
Changes from all commits
002bcf0
c0e2352
6e6ec88
412e191
e2cf2b5
7aca47c
601231f
51b6979
92cf2a6
68a3893
b4f8adf
1112ae6
feb8020
40bd151
1a78dfd
b7fd431
65a6b48
5c9738e
016ad53
9b7d7c1
77b853d
c42584c
3f40a65
2a0d68b
40eabd8
02d1ddd
298193d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
I just noticed this line is incorrect, we should use
!_.isEmpty(tagList)
instead of!!tagList
. @BeeMargarida can you add that change to your next PR?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.
Yap!
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.
I don't think this copy was checked? It does not really read like a proper English sentence. We're going to create an issue to modify it.
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.
It's the copy that was in the mocks in the design doc, but yeah I am not sure if it was run through marketing prior to landing in the mocks (I did not run it through after, that much I know 😅). I agree it's a bit awkward wording and could be improved.
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.
Fixed here: #38242