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
New timezone pages #12201
New timezone pages #12201
Changes from all commits
9704e1e
c9cad55
bcae580
b173e21
f1bc673
cfbdb16
8e3bf2e
b9fd35a
2221e56
06dfa1d
05e67bf
d2afbd9
c2c7e70
9e6eb14
45951b4
c51cc57
a7758c8
c02e802
cf1ca69
5213617
97df428
d61aac0
467760b
818ac83
f13ab5f
1ab9ee9
4c1910e
2be55f9
717fbc4
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.
NAB - style: maybe it'll be easier to read the constructor if we create a
getAllTimeZones()
function in this class.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.
Hi all! When we added this new feature of adding a checkmark, we created an inconsistency - Other pages like currency selection didn't get any checkmarks making them unhappy.
It was out of scope of this PR, but we should have created a follow up issue for it!
Thanks for reading :)
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.
Oooooo fair point, thanks for noting that! We always like happy pages and happy code
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.
@Beamanator
thought: instead of maintaining a state for timezoneOptions, could we pass in a filtered array?
it's gonna re-render when we type.
feel free to ignore if it doesn't make the code cleaner.