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 #1675: Relocate button "I can't give feedback" #1706

Merged
merged 10 commits into from
May 13, 2022

Conversation

Kakadus
Copy link
Collaborator

@Kakadus Kakadus commented Feb 14, 2022

Fixes #1675

Copy link
Member

@janno42 janno42 left a comment

Choose a reason for hiding this comment

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

button location and functionality are perfect. unfortunately, the keyboard navigation doesn't work anymore, tabbing over the repositioned button is currently not possible. @niklasmohrin can probably help you with that :)

Kakadus and others added 2 commits March 1, 2022 08:19
Co-authored-by: Niklas Mohrin <dev@niklasmohrin.de>
Copy link
Member

@richardebeling richardebeling left a comment

Choose a reason for hiding this comment

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

Code looks good, behavior seems fine. Thanks!

@janno42 when using the keyboard to select "I can't give feedback [...]", the keyboard focus stays on this button while the card collapses. So, to advance, users have to press "tab" afterwards. Until they do, they will not see which item has keyboard focus. My first intuition was that it should automatically forward to the next element, but the more I think about it, the less I think I'd like it. What do you think?

@richardebeling richardebeling requested a review from janno42 March 13, 2022 16:04
@janno42
Copy link
Member

janno42 commented Mar 14, 2022

Code looks good, behavior seems fine. Thanks!

@janno42 when using the keyboard to select "I can't give feedback [...]", the keyboard focus stays on this button while the card collapses. So, to advance, users have to press "tab" afterwards. Until they do, they will not see which item has keyboard focus. My first intuition was that it should automatically forward to the next element, but the more I think about it, the less I think I'd like it. What do you think?

For me, both solutions are fine. Because keeping it as it is now produces less overhead, I'd say it can stay as it is :)

Copy link
Member

@janno42 janno42 left a comment

Choose a reason for hiding this comment

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

✔️ Meets requirements
✔️ UI functionality checked

Copy link
Member

@niklasmohrin niklasmohrin left a comment

Choose a reason for hiding this comment

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

(blocking for my comment earlier)

@janno42
Copy link
Member

janno42 commented May 9, 2022

@Kakadus, do you want to continue working on this PR? There shouldn't be much left to do here.

@Kakadus
Copy link
Collaborator Author

Kakadus commented May 9, 2022

I am refactoring it now...

@janno42 janno42 requested a review from niklasmohrin May 9, 2022 18:41
@Kakadus Kakadus force-pushed the relocate-button branch from 690d502 to 71900b6 Compare May 9, 2022 20:47
@janno42
Copy link
Member

janno42 commented May 13, 2022

Thanks! :shipit:

You could have a look at your git settings, you are committing with three different users, two having invalid email addresses, one of which ends in "guthub.com" :)

@janno42 janno42 merged commit a3ca8bc into e-valuation:main May 13, 2022
Kakadus added a commit to Kakadus/EvaP that referenced this pull request Jul 25, 2022
…ation#1706)

* relocate-button

* make tabbing work correctly

* fix js pitfall

Co-authored-by: Niklas Mohrin <dev@niklasmohrin.de>

* move first entry in loop

* first reformat

* extract hasTabbingTarget

* Update student-vote.ts
Co-authored-by: Niklas Mohrin <dev@niklasmohrin.de>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Relocate button "I can't give feedback"
4 participants