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
feat: Add set first day week android #902
feat: Add set first day week android #902
Changes from 12 commits
22d4805
c240ded
e2cbf3d
db0a276
9f8f3c9
a20de55
43a0511
6858793
9759027
f344787
46a4888
8d05525
11ea194
0f9786b
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.
You mentioned you needed to add scrolling. Can we make this more compact in terms of height?
I'd prefer if we can avoid scrolling because it's annoying to think about it when maintaining tests.
Also the code seems fit to be replaced by a
array.map
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.
First off, thank you for the lovely comments you've made, they made my day! 😄
Now, with regards to this, Ive implemented a FlatList design for selecting the
DAY_OF_WEEK
variable (similar to the "timezone" FlatList further down) but, sadly, it seems most of the tests would still need to have some level of scrolling to access the UI elements... Either way I will push the changes shortly and you can check them out!I've gone through most of the tests and I think it might be reasonable to implement a "ScrollToElement" function that allows you to scroll to the necessary UI element before interacting with it. This would also reduce the size and repetitiveness of some of the tests and hopefully aid with maintainability. I was thinking of maybe creating a separate PR going over them and addressing some of the issues. Let me know what you think!
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.
Sounds good to me, though if we can avoid scrolling, that'd be best imho. I see that now we run in iphone 14. A low-effort fix could be just bumping to a larger device.
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 will revert the changes to the tests that add scrolling and see if I can find a valid device for iOS and Android that will pass the tests without scrolling, will get back to you shortly