Skip to content
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

fix: ensure focused month is visible when receiving focus #3219

Merged
merged 8 commits into from
Jan 7, 2022

Conversation

vursen
Copy link
Contributor

@vursen vursen commented Dec 24, 2021

Description

This PR fixes the bug where a month receiving focus by the keyboard or programmatically sometimes wasn't scrolled into the visible area of the calendar.

An investigation established that the bug is caused by the buffer offset that can be set with the --vaadin-infinite-scroller-buffer-offset CSS variable. This offset isn't taken into account by the revealDate method which leads to computing an erroneously greater number of visible months when an offset is set and, as a result, scrolling doesn't happen. Worth noting that the buffer offset is a non-zero value in both Lumo and Material themes so both themes are affected.

💡tip: When reviewing this PR, enable the "Hide whitespace" option to reduce the diff.

Fixes #1892

Type of change

  • Bugfix

Checklist

  • I have read the contribution guide: https://vaadin.com/docs-beta/latest/guide/contributing/overview/
  • I have added a description following the guideline.
  • The issue is created in the corresponding repository and I have referenced it.
  • I have added tests to ensure my change is effective and works as intended.
  • New and existing tests are passing locally with my change.
  • I have performed self-review and corrected misspellings.

@vursen vursen changed the title fix: scroll calendar to keep focused month visible fix: scroll calendar to get focused month visible Dec 24, 2021
@vursen vursen changed the title fix: scroll calendar to get focused month visible fix: scroll calendar to make focused month visible Dec 24, 2021
@vursen vursen changed the title fix: scroll calendar to make focused month visible fix: scroll calendar when focused month is not fully visible Dec 24, 2021
@vursen vursen changed the title fix: scroll calendar when focused month is not fully visible fix: ensure focused month is visible when receiving focus Dec 24, 2021
@vursen vursen force-pushed the fix/date-picker/reveal-date branch from 5c825d6 to c436092 Compare January 5, 2022 17:24
@vursen vursen marked this pull request as ready for review January 6, 2022 07:45
@vursen vursen force-pushed the fix/date-picker/reveal-date branch from 59d11ff to 1532c89 Compare January 7, 2022 10:06
@sonarcloud
Copy link

sonarcloud bot commented Jan 7, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 4 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@web-padawan web-padawan merged commit 9a0c2fd into master Jan 7, 2022
@web-padawan web-padawan deleted the fix/date-picker/reveal-date branch January 7, 2022 11:32
@vaadin-bot
Copy link
Collaborator

Hi @vursen , this commit cannot be picked to 22.0 by this bot, can you take a look and pick it manually?
Error Message: Error: Command failed: git cherry-pick 9a0c2fd
error: could not apply 9a0c2fd... fix: ensure focused month is visible when receiving focus (#3219)
hint: after resolving the conflicts, mark the corrected paths
hint: with 'git add ' or 'git rm '
hint: and commit the result with 'git commit'

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[date-picker] The focus-ring on the focused day is getting hidden while pressing Arrow Down
5 participants