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

Remove vendored copy of fomantic-dropdown #15193

Merged
merged 3 commits into from
Mar 30, 2021
Merged

Conversation

silverwind
Copy link
Member

@silverwind silverwind commented Mar 29, 2021

jQuery 3.6.0 seems to have broke the dropdown focus handling (focus would get stuck on the dropdown) in this module which we have vendored on top of fomantic for accessibility improvements.

Either downgrading jQuery to 3.5.1 or removing the vendor copy seems to resolve the issue and I opted for removing the copy because I think such changes should be done upstream and the removal also lightens the JS by 155kB before minify/gzip.

Fixes: #15172
Ref: #8638
Ref: jquery/jquery#4813

jQuery 3.6.0 seems to have broke the dropdown focus handling (focus
would get stuck on the dropdown) in this module which we have vendored
on top of fomantic for accessibility improvements.

Either downgrading jQuery to 3.5.1 or removing the vendor copy seems to
resolve the issue and I opted for removing the copy because I think such
changes should be done upstream and the removal also lightens the JS by
155kB before minify/gzip.

Fixes: go-gitea#15172
@silverwind silverwind added type/bug topic/ui Change the appearance of the Gitea UI kind/usability labels Mar 29, 2021
@silverwind silverwind added this to the 1.15.0 milestone Mar 29, 2021
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Mar 29, 2021
Copy link
Member

@a1012112796 a1012112796 left a comment

Choose a reason for hiding this comment

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

👍

@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 Mar 30, 2021
@techknowlogick
Copy link
Member

Have these a11y fixes been upstreamed? I don't want to lose any accessibility by merging this.

@silverwind
Copy link
Member Author

silverwind commented Mar 30, 2021

No, it was never followed up and it seems the original author also has no interest in upstreaming the patch. So yes, we'd loose some a11y with this removal, but I think it's overall a net gain to no longer have this crude way of patching fomantic (essentially it just runs another copy of the fomantic module on top of the already initialized original module, a recipe for disaster).

@6543 6543 merged commit 1b762fc into go-gitea:master Mar 30, 2021
@silverwind silverwind deleted the fix-dropdown branch March 30, 2021 21:21
@henningoschwald
Copy link

As expected this definitely breaks a11y for me using the Orca screenreader on Linux with Firefox, i.e. in the "create" drop-down in the main navigation. Tested with Gitea master from April 19th. Other browsers and screenreaders are affected too, of course. I really hope you don't intend to stick with this state and I wish you would give a11y a higher priority in general.

I am not deep enough into web development to come up with pull requests, but I would be available as a tester and I am able to build Gitea from sources.

@silverwind
Copy link
Member Author

silverwind commented Apr 26, 2021

We are deeply dependant on Fomantic UI which was not created with accessiblity in mind. I think our best bet is to gradually replace it with something better, but I'm not sure what to replace it with.

Alternatively, we could again try to hack something up to add aria attributes after Dropdown creation via DOM modifaction. If someone could provide a example HMTL diff with such attributes, I'm sure something could be done.

@go-gitea go-gitea locked and limited conversation to collaborators May 13, 2021
@delvh delvh added topic/ui-interaction Change the process how users use Gitea instead of the visual appearance and removed topic/ui Change the appearance of the Gitea UI kind/usability labels Oct 8, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. topic/ui-interaction Change the process how users use Gitea instead of the visual appearance type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

When selecting a license you can't write in any field
8 participants