fix(datetime): scroll failing for adjacent days on ios#31033
fix(datetime): scroll failing for adjacent days on ios#31033os-davidlourenco merged 2 commits intomainfrom
Conversation
os-davidlourenco
added
package: core
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
ShaneK
left a comment
ShaneK
left a comment
There was a problem hiding this comment.
A good start, but I think this might not be the best solution yet
| left: 0, | ||
| behavior: 'smooth', | ||
| }); | ||
| prevMonth.scrollIntoView({ behavior: 'smooth' }); |
There was a problem hiding this comment.
scrollIntoView({ behavior: 'smooth' }) defaults to block: 'start', which scrolls ancestors vertically to align the element to the top of each scrollable viewport. The old scrollTo({ top: 0 }) was scoped to the calendar container. Adding block: 'nearest' would at least reduce the vertical scroll disturbance here. Also, the existing adjacent-days test only exercises the nextMonth() path (clicking a November adjacent day from October). Would it be possible to add a test that covers this fix directly? Something like: navigate back a month first, then click an adjacent day from the month before that, and assert the header title and ionChange value.
There was a problem hiding this comment.
To ensure this works in iOS, a calculation and approach similar to the next was developed in the previous. I also added a test to validate this behavior
os-davidlourenco
force-pushed
the
ROU-12668
branch
from
1a69a49 to
28c4acb
Compare
os-davidlourenco
force-pushed
the
ROU-12668
branch
from
28c4acb to
dad53e0
Compare
os-davidlourenco
force-pushed
the
ROU-12668
branch
from
dad53e0 to
a31502b
Compare
ShaneK
left a comment
ShaneK
left a comment
There was a problem hiding this comment.
This looks good to me! The only edge case I can think of that may remain is if you click on an adjacent day twice in a row, the second one before the swipe for the current one finishes settling. That feels like an unlikely edge case and may be out of scope for you to try fixing here, but just wanted to point it out in case you think otherwise
Issue number: resolves internal --------- <!-- Please do not submit updates to dependencies unless it fixes an issue. --> <!-- Please try to limit your pull request to one type (bugfix, feature, etc). Submit multiple pull requests if needed. --> When the user swipes to a previous month that has adjacent days and selects one of the adjacent days from a previous month, the datetime selects the day but fails to scroll to the previous month. When a user clicks on a previous adjacent day after swiping the month, the calendar scrolls to the previous month. The same is applied to the next month. - [ ] Yes - [x] No <!-- If this introduces a breaking change: 1. Describe the impact and migration path for existing applications below. 2. Update the BREAKING.md file with the breaking change. 3. Add "BREAKING CHANGE: [...]" to the commit description when merging. See https://github.com/ionic-team/ionic-framework/blob/main/docs/CONTRIBUTING.md#footer for more information. --> The previous behavior was only reproducible on iOS real devices.
2 participants
Issue number: resolves internal
What is the current behavior?
When the user swipes to a previous month that has adjacent days and selects one of the adjacent days from a previous month, the datetime selects the day but fails to scroll to the previous month.
What is the new behavior?
When a user clicks on a previous adjacent day after swiping the month, the calendar scrolls to the previous month. The same is applied to the next month.
Does this introduce a breaking change?
Other information
The previous behavior was only reproducible on iOS real devices.