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

Improve accessibility when (re-)viewing files #24817

Merged
merged 12 commits into from May 21, 2023
Merged

Improve accessibility when (re-)viewing files #24817

merged 12 commits into from May 21, 2023

Conversation

delvh
Copy link
Member

@delvh delvh commented May 19, 2023

Visually, nothing should have changed.
Changes include

  • Convert most <a [no href]> to <button> when (re-)viewing files:
    • <a [no href]> are, by HTML definition, not a link and hence cannot be focused
  • <a class="ui button"> can now be clicked (again?) using Enter
    • Previously, the installed keypress handler on .ui.button elements disabled it for links somehow
  • The (un)escape file, the expand section and the expand/collapse file buttons can now be focused (and subsequently clicked using only the keyboard)
  • You can now press Space on a focused View file checkbox to mark the file as viewed.
    • previously, this was impossible as this checkbox listened on the wrong event listener

The add code comment button has been left inaccessible for now as it requires quite a bit of extra logic so that it is unhidden when it is focused (you can otherwise focus it without seeing it as you are not hovering on the corresponding line).

@delvh delvh added topic/pr Issues related to pull requests topic/accessibility This issue/pull request wants to improve the accessibility labels May 19, 2023
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label May 19, 2023
@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label May 19, 2023
silverwind
silverwind reviewed May 19, 2023
View reviewed changes
web_src/css/helpers.css Outdated Show resolved Hide resolved
@pull-request-size pull-request-size bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels May 20, 2023
delvh and others added 8 commits May 20, 2023 12:03
@delvh @silverwind
Improve accessibility when (re-)viewing files
8039a09 
Visually, nothing should have changed.
Changes include
- Convert most `<a [no href]>` to `<button>` when (re-)viewing files
- `<a class="ui button">` can now be clicked (again?) using <kbd>Enter</kbd>
- The `(un)escape file`, the `expand section` and the `expand/collapse file`
  buttons can now be focused (and subsequently clicked using only the keyboard)
- Previously, pressing <kbd>Space</kbd> on the `View file` checkbox did
  nothing as this checkbox listened to the wrong event listener.
  Now, you can use the keyboard for this checkbox too.

The `add code comment` button has been left inaccessible for now as it
requires quite a bit of extra logic so that it is unhidden when it is
focused (you can otherwise focus it without seeing it as you are not
hovering on the corresponding line).
@delvh @silverwind
Remove inline style and unused helper class as per @silverwind
d9db226
@silverwind
Update templates/repo/diff/box.tmpl
840f20a
@delvh @silverwind
Fix now incorrect CSS selectors, add button-ghost for a missed escape
fc2c7e9
@delvh @silverwind
Wrap the split expand section buttons in a flexbox too
919d6e3
@delvh @silverwind
Use `` instead of "" and revert unrelated change as per @wxiaoguang
ac6a6ae
@delvh @silverwind
Rewrite key handler for .ui.button to only capture <span> and `<d…
abd9756 
…iv>`
@delvh @silverwind
Center the newly created excerpts too
29733fd
@silverwind silverwind force-pushed the a11y/convert-a-without-href-to-buttons branch from a6a7967 to 140b366 Compare May 20, 2023 10:04
@pull-request-size pull-request-size bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels May 20, 2023
@silverwind
Copy link
Member

Sorry, pushed some unrelated commits, rebasing...

@silverwind silverwind force-pushed the a11y/convert-a-without-href-to-buttons branch from 140b366 to 306767b Compare May 20, 2023 10:05
@silverwind

This comment was marked as resolved.

Sign in to view
@delvh

This comment was marked as resolved.

Sign in to view
@delvh
Copy link
Member Author

delvh commented May 20, 2023

Okay, apparently my make watch was somehow broken.
I have no idea how, but somehow it seems to have cached old CSS, even across browser cache clears.
I've now restarted make watch, and suddenly it looks like it looks for you.

@delvh
Copy link
Member Author

delvh commented May 20, 2023

@silverwind @wxiaoguang anything else to do here?

@silverwind
Copy link
Member

Don't think so, but I will give it one final look a bit later.

wxiaoguang
wxiaoguang approved these changes May 21, 2023
View reviewed changes
Copy link
Contributor

@wxiaoguang wxiaoguang left a comment

Choose a reason for hiding this comment

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

Overall LGTM. Wait for silverwind's final look 😄

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels May 21, 2023
@delvh
Copy link
Member Author

delvh commented May 21, 2023

@silverwind I do have to say, that's a pretty long final look… 😁

@wxiaoguang
Copy link
Contributor

Maybe not that long, it hasn't been one day yet 😄 If I am busy on other things, I also would be away from keyboard for several days.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels May 21, 2023
@silverwind silverwind added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label May 21, 2023
@silverwind silverwind enabled auto-merge (squash) May 21, 2023 20:14
@silverwind silverwind merged commit e95b42e into go-gitea:main May 21, 2023
15 checks passed
@GiteaBot GiteaBot added this to the 1.20.0 milestone May 21, 2023
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label May 21, 2023
@delvh delvh deleted the a11y/convert-a-without-href-to-buttons branch May 21, 2023 21:03
silverwind added a commit to silverwind/gitea that referenced this pull request May 21, 2023
@silverwind
Merge branch 'main' into font-600
77c4599 
* main:
  Rewrite logger system (go-gitea#24726)
  Support Copy Link for video attachments (go-gitea#24833)
  Fix video width overflow in markdown, and other changes to match img (go-gitea#24834)
  Improve accessibility when (re-)viewing files (go-gitea#24817)
zjjhot added a commit to zjjhot/gitea that referenced this pull request May 22, 2023
@zjjhot
Merge remote-tracking branch 'giteaofficial/main'
6791aa3 
* giteaofficial/main: (27 commits)
  Fix regression: access log template, gitea manager cli command (go-gitea#24838)
  Merge message template support for rebase without merge commit (go-gitea#22669)
  [skip ci] Updated licenses and gitignores
  Support wildcard in email domain allow/block list (go-gitea#24831)
  Change `--font-weight-bold` to `--font-weight-semibold` and 600 value, introduce new font weight variables (go-gitea#24827)
  Rewrite logger system (go-gitea#24726)
  Support Copy Link for video attachments (go-gitea#24833)
  Fix video width overflow in markdown, and other changes to match img (go-gitea#24834)
  Improve accessibility when (re-)viewing files (go-gitea#24817)
  Refactor rename user and rename organization (go-gitea#24052)
  Use `CommentList` instead of `[]*Comment` (go-gitea#24828)
  Fix topics deleted via API not being deleted in org page (go-gitea#24825)
  Return `404` in the API if the requested webhooks were not found (go-gitea#24823)
  Decouple the different contexts from each other (go-gitea#24786)
  [skip ci] Updated translations via Crowdin
  Add RTL rendering support to Markdown (go-gitea#24816)
  [skip ci] Updated translations via Crowdin
  Update JS dependencies (go-gitea#24815)
  Fix duplicate tooltip hiding (go-gitea#24814)
  Mute repo names in dashboard repo list (go-gitea#24811)
  ...
@delvh delvh mentioned this pull request Jul 14, 2023
Merged
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Aug 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@silverwind silverwind silverwind approved these changes

@wxiaoguang wxiaoguang wxiaoguang approved these changes

Assignees
No one assigned
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. topic/accessibility This issue/pull request wants to improve the accessibility topic/pr Issues related to pull requests
Projects
None yet
Milestone
1.20.0
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@delvh @silverwind @wxiaoguang @GiteaBot