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

feat(a11y): Change behavior of some file upload fields #1577

Merged

Conversation

jsit
Copy link
Contributor

@jsit jsit commented Jun 25, 2023

Several file upload fields were hiding the main <input type="file"> and relying on just the label for clicking.

This made these fields inaccessible to tabbing, and possibly some other negative effects of hiding native elements. <label> can have a tabindex, but evidently that's not a good practice.

In a couple places here I've just added the form-control class to the inputs, which Bootstrap styles nicely.

The places I didn't touch this are the emoji upload page, where there isn't much space, and the Markdown toolbar, where there isn't any room. (I added an eslint-disable to ignore the tabindex being on the label for keyboard access for those.)

I cleaned up the Create Post page to make image uploading a little clearer, too.

Another thing I noticed is that in some cases these upload fields were a <form> inside of a <form>, which they don't have to be.

Affected pages are:

  1. Create Post
  2. Personal Settings
  3. Create Community
  4. Edit Site

Create Post

Before

Screenshot 2023-06-25 at 2 33 03 AM

After

Screenshot 2023-06-26 at 6 38 48 PM

User Settings

Before

Screenshot 2023-06-25 at 2 32 24 AM

After

Screenshot 2023-06-26 at 6 38 43 PM

Create Community

Before

Screenshot 2023-06-25 at 2 32 06 AM

After

Screenshot 2023-06-26 at 6 38 55 PM

Site Settings

Before

Screenshot 2023-06-25 at 2 31 47 AM

After

Screenshot 2023-06-26 at 6 39 00 PM

Custom Emoji

After

Screenshot 2023-06-25 at 2 25 52 AM

Avatar aspect ratios

This PR also fixes avatar aspect ratios

Before

Screenshot 2023-07-02 at 5 26 45 PM

After

Screenshot 2023-07-02 at 5 21 38 PM Screenshot 2023-07-02 at 5 31 09 PM

jsit added 2 commits June 25, 2023 02:27
…upload-a11y

* lemmy/main:
  fix: Add type=button to buttons
  chore: Empty commit to re-trigger Woodpecker
  fix: Fix some Bootstrap 5 font classes
  fix: Fix some Bootstrap 5 font classes
  fix: Specify vote content type so buttons work for both comments and posts
  v0.18.0
  Fix homepage `scrollTo(0, 0)` failing when document size changes.
  v0.18.0-rc.8
  Moved `!isBrowser()` check to `FirstLoadServer.isFirstLoad`
  Fix server-side rendering after first load.
  fix!: Try to get Vote Buttons component working in Comments
  fix: Remove unused prop
  fix: Rework some vote buttons architecture
  fix: Undo some other extraneous changes
  fix: Undo some extraneous changes
  fix: Remove tippy duplicate functions
  fix: Revert to old mobile vote style
  feat: Move vote buttons to separate component
src/assets/css/main.css Outdated Show resolved Hide resolved
jsit and others added 21 commits June 26, 2023 18:30
* Fix: missing semantic css classes and html elements.

Now all pages have a main and aside element when a sidebar is present to facilitate custom theming. This does not impact the default behavior of the front.

* Fix: re-added communityref on main element

---------

Co-authored-by: 0xAnansi <[email protected]>
Co-authored-by: Jay Sitter <[email protected]>
Co-authored-by: Dessalines <[email protected]>
* feat: Reduce base font size

* chore: Build themes

---------

Co-authored-by: SleeplessOne1917 <[email protected]>
Co-authored-by: Dessalines <[email protected]>
* rework password reset form

* make self-suggested changes

* cleaning

* validate in handlePasswordReset as well

* update submodule

* partially make suggested changes

* make suggested changes

* resolve merge conflicts

* resolve merge conflicts

* resolve merge conflicts

---------

Co-authored-by: Dessalines <[email protected]>
* Use was-validated class in signup form

* Update signup.tsx

* Update signup.tsx

---------

Co-authored-by: SleeplessOne1917 <[email protected]>
 When banning or unbanning, the API call was done, but updating the
frontend failed. This caused a confusing experience for an admin, until
the page was reloaded.
@jsit jsit requested a review from dessalines June 26, 2023 22:43
Copy link
Member

@SleeplessOne1917 SleeplessOne1917 left a comment

Choose a reason for hiding this comment

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

Better than what it used to be, but it could use a few more touches before shipping.

For the site settings, could use a bit of margin between the images and the file inputs. I'm also not a fan of the tiny "x" on the image to remove the image.
image

For create post and create community, a bit of margin between the uploader and image would be good:
image

For edit community and user settings, same critiques as the site settings page apply:
image
image

@alectrocute
Copy link
Contributor

I don’t know if it’s related, but the avatar/icons that have a weird aspect ratio (not a circle) has been hurting me for awhile. Could there be an opportunity to fix that here? 😌

@jsit
Copy link
Contributor Author

jsit commented Jun 27, 2023

@alectrocute

I don’t know if it’s related, but the avatar/icons that have a weird aspect ratio (not a circle) has been hurting me for awhile. Could there be an opportunity to fix that here? 😌

10,000,000%

@jsit
Copy link
Contributor Author

jsit commented Jun 27, 2023

Thanks for battle-testing this @SleeplessOne1917, I probably won't get to this today.

@jsit
Copy link
Contributor Author

jsit commented Jul 2, 2023

@SleeplessOne1917 I'm gonna work on those spacing issues before merging this

@jsit
Copy link
Contributor Author

jsit commented Jul 2, 2023

@SleeplessOne1917 Fixed spacing between image and file input:

Screenshot 2023-07-02 at 5 14 21 PM Screenshot 2023-07-02 at 5 31 51 PM Screenshot 2023-07-02 at 5 14 17 PM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants