Skip to content

Conversation

@eps1lon
Copy link
Member

@eps1lon eps1lon commented Jan 8, 2021

This PR should not change "intended" keyboard navigation. Its goal is to simplify focus managment and re-use existing patterns.

  • remove usage of useGlobalKeyDown for keyboard navigation of Day and YearPicker
    It's unclear why we need global event handlers. The only focusable items in the collection are the individual days/years so we might as well attach the listener to these elements directly. React takes care of event delegation.
  • remove usage of useCanAutoFocus
    Seems to me this works just as well by assuming that "button isMounted? => canAutoFocus"

Manual testing:

component next PR
TimePicker 602baa7774efe900072cd914 deploy-preview-24315
DatePicker 602baa7774efe900072cd914 deploy-preview-24315
DateTimePicker 602baa7774efe900072cd914 deploy-preview-24315
DateRangePicker 602baa7774efe900072cd914 deploy-preview-24315

test_profile baseline Analysis: https://mui-dashboard.netlify.app/test-profile/225337
test_profile PR Analysis: https://mui-dashboard.netlify.app/test-profile/225317

@mui-pr-bot
Copy link

mui-pr-bot commented Jan 8, 2021

@material-ui/lab: parsed: -0.06% 😍, gzip: -0.15% 😍

Details of bundle changes

Generated by 🚫 dangerJS against f64e37e

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Jan 11, 2021
@eps1lon eps1lon force-pushed the fix/lab/pickers-gdsfp branch from 9dd7afc to 64f02f0 Compare January 12, 2021 07:49
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged. label Jan 12, 2021
@eps1lon eps1lon changed the title [DatePicker] Move cascading update into render instead of effect phase [DatePicker] Simplify useCanAutoFocus Jan 12, 2021
@eps1lon eps1lon force-pushed the fix/lab/pickers-gdsfp branch from 7fa5485 to 30039ca Compare January 14, 2021 13:18
@eps1lon eps1lon force-pushed the fix/lab/pickers-gdsfp branch 2 times, most recently from efd8e03 to 67d9eef Compare February 1, 2021 13:40
@eps1lon eps1lon force-pushed the fix/lab/pickers-gdsfp branch from 67d9eef to 760c953 Compare February 2, 2021 13:41
@eps1lon eps1lon changed the title [DatePicker] Simplify useCanAutoFocus [pickers] Rework keyboard navigation implementation Feb 2, 2021
@eps1lon eps1lon added scope: pickers Changes related to the date/time pickers. and removed component: DatePicker The React component. labels Feb 2, 2021
@eps1lon eps1lon force-pushed the fix/lab/pickers-gdsfp branch from 143fd09 to 9487537 Compare February 3, 2021 10:20
@eps1lon eps1lon marked this pull request as ready for review February 3, 2021 11:58
@eps1lon eps1lon mentioned this pull request Feb 4, 2021
2 tasks
@oliviertassinari
Copy link
Member

@eps1lon oops, sorry for the delay, I will dive into this one tomorrow

@eps1lon
Copy link
Member Author

eps1lon commented Feb 11, 2021

@eps1lon oops, sorry for the delay, I will dive into this one tomorrow

All good. I have quite the backlog myself so this will have two sets of fresh eyes 😉

@eps1lon eps1lon force-pushed the fix/lab/pickers-gdsfp branch from fcda0aa to f64e37e Compare February 16, 2021 14:27
tchock added a commit to Bojagi/material-ui that referenced this pull request Feb 16, 2021
This is a copy of upstream PR mui#24315
tchock added a commit to Bojagi/material-ui that referenced this pull request Feb 16, 2021
This is a copy of upstream PR mui#24315
tchock added a commit to Bojagi/material-ui that referenced this pull request Feb 16, 2021
This is a copy of upstream PR mui#24315
@eps1lon
Copy link
Member Author

eps1lon commented Feb 19, 2021

Merging since this simplifies the implementation considerably. I'd like to get this in as well as click-away fixes until monday.

@eps1lon eps1lon merged commit 6cba359 into mui:next Feb 19, 2021
@eps1lon eps1lon deleted the fix/lab/pickers-gdsfp branch February 19, 2021 19:29
@oliviertassinari oliviertassinari changed the title [pickers] Rework keyboard navigation implementation [Pickers] Rework keyboard navigation implementation Feb 27, 2021
tchock added a commit to Bojagi/material-ui that referenced this pull request Mar 10, 2021
This is a copy of upstream PR mui#24315
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance scope: pickers Changes related to the date/time pickers.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants