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.

Sign up for GitHub

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

Jump to bottom

ENH Improve component focusing #260

Merged
merged 1 commit into from
Apr 4, 2024
Merged

ENH Improve component focusing #260

merged 1 commit into from
Apr 4, 2024

Conversation

emteknetnz
Copy link
Member

@emteknetnz emteknetnz commented Mar 21, 2024 •
edited
Loading

Issue #245

This PR:

  • Makes focus when using mouse and keyboard consistent. Previously only using the keyboard has special logic for setting the focus state
  • Fixes a timing issue with focus when using a keyboard
  • Swaps around the tab order of the drag handle so that it is last rather than first

@emteknetnz emteknetnz force-pushed the pulls/4/a-focus branch 2 times, most recently from 05eb35c to a829c41 Compare March 21, 2024 22:57
@emteknetnz emteknetnz marked this pull request as draft March 21, 2024 23:37
@emteknetnz emteknetnz mentioned this pull request Mar 21, 2024
Closed
@emteknetnz emteknetnz force-pushed the pulls/4/a-focus branch 5 times, most recently from 42ad51a to 524aa9b Compare March 27, 2024 03:24
@emteknetnz emteknetnz marked this pull request as ready for review March 27, 2024 03:30
@emteknetnz emteknetnz mentioned this pull request Mar 27, 2024
Open
GuySartorelli
GuySartorelli requested changes Apr 2, 2024
View reviewed changes
Copy link
Member

@GuySartorelli GuySartorelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When I archive a link and then reorder links via the keyboard I get this error in the console:

Assertion failed: Input argument is not an HTMLInputElement

I don't get that if I reorder with the mouse.

client/src/components/LinkModal/LinkModal.js Show resolved Hide resolved
@GuySartorelli
Copy link
Member

Other than the specific problems above, it works great locally.

@emteknetnz emteknetnz force-pushed the pulls/4/a-focus branch 2 times, most recently from ad93b5e to 946d3f1 Compare April 2, 2024 23:04
@emteknetnz
Copy link
Member Author

When I archive a link and then reorder links via the keyboard I get this error in the console:

I'm unable to replicate this. Could you please double check this is still happening with the latest version of this PR and if so then demo it to me.

@GuySartorelli
Copy link
Member

Can't replicate that problem anymore, no idea what was causing it.
LGTM but has merge conflicts.

@emteknetnz
Copy link
Member Author

Done

GuySartorelli
GuySartorelli approved these changes Apr 4, 2024
View reviewed changes
Copy link
Member

@GuySartorelli GuySartorelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@GuySartorelli GuySartorelli merged commit ecc7c67 into silverstripe:4 Apr 4, 2024
12 checks passed
@GuySartorelli GuySartorelli deleted the pulls/4/a-focus branch April 4, 2024 01:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@GuySartorelli GuySartorelli GuySartorelli approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@emteknetnz @GuySartorelli