-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
New Pronouns page #12301
New Pronouns page #12301
Conversation
Waiting a bit on this convo: https://expensify.slack.com/archives/C01GTK53T8Q/p1667832298808449?thread_ts=1666881832.702589&cid=C01GTK53T8Q |
No longer on hold, PR still in progress |
sheHerHers: 'She/her', | ||
theyThemTheirs: 'They/them', | ||
zeHirHirs: 'Ze/hir', | ||
selfSelect: 'Self-select', |
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.
Note: Removing this for now, as we'll come back to "self-select" / pronoun customization later
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 comment was marked as duplicate.
This comment was marked as duplicate.
2 similar comments
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
This comment was marked as duplicate.
@thesahindia Thanks for testing so far! I like the idea of making the page scroll to the selected pronoun, I think I'd prefer taking this as a follow-up tweak (and we can ask design team in Slack what they think) Also, for the name glitch - that's pretty weird, but luckily we're going to remove display name, timezone, and pronouns data from the Profile Page once all these new pages are done - so I don't think we need to worry about that for now 👍 |
Looks good and tests well 🎉 C+ reviewed 🎀👀🎀
|
Thanks @thesahindia ! All you @robertjchen 👍 |
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.
Looks great! 🎉
@robertjchen looks like this was merged without the checklist test passing. Please add a note explaining why this was done and remove the |
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
🚀 Deployed to staging by @robertjchen in version: 1.2.29-0 🚀
|
🚀 Deployed to production by @AndrewGable in version: 1.2.29-7 🚀
|
zeHirHirs: 'Ze/hir', | ||
selfSelect: 'Self-select', | ||
callMeByMyName: 'Call me by my name', | ||
}, |
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.
Coming from #38969.
It seems that we didn't migrate old values in DB, causing a MISSING_TRANSLATION error, we added a PronounsMigration.ts
file to upgrade these values. :)
Details
Command being created here: https://github.com/Expensify/Web-Expensify/pull/35315- Done!Fixed Issues
$ #11576
Tests
Offline
QA Steps
Offline
PR Review Checklist
PR Author Checklist
### Fixed Issues
section aboveTests
sectionQA steps
sectiontoggleReport
and notonIconClick
)src/languages/*
filesWaiting for Copy
label for a copy review on the original GH to get the correct copy.STYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)/** comment above it */
this
properly so there are no scoping issues (i.e. foronClick={this.submit}
the methodthis.submit
should be bound tothis
in the constructor)this
are necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);
ifthis.submit
is never passed to a component event handler likeonClick
)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)PR Reviewer Checklist
The reviewer will copy/paste it into a new comment and complete it after the author checklist is completed
### Fixed Issues
section aboveTests
sectionQA steps
sectiontoggleReport
and notonIconClick
).src/languages/*
filesWaiting for Copy
label for a copy review on the original GH to get the correct copy.STYLE.md
) were followedAvatar
, I verified the components usingAvatar
have been tested & I retested again)/** comment above it */
this
properly so there are no scoping issues (i.e. foronClick={this.submit}
the methodthis.submit
should be bound tothis
in the constructor)this
are necessary to be bound (i.e. avoidthis.submit = this.submit.bind(this);
ifthis.submit
is never passed to a component event handler likeonClick
)StyleUtils.getBackgroundAndBorderStyle(themeColors.componentBG
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Screenshots
Web
Web & Mobile Web - Chrome
Screen.Recording.2022-11-14.at.4.54.00.PM.mov
Web & Mobile Web - Safari
Screen.Recording.2022-11-14.at.4.18.03.PM.mov
Desktop
N/A (can't manually navigate to new pronouns page)
iOS
N/A (can't manually navigate to new pronouns page)
Android
N/A (can't manually navigate to new pronouns page)