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

Fix new album form layout #2099

Merged
merged 2 commits into from
Oct 23, 2023
Merged

Fix new album form layout #2099

merged 2 commits into from
Oct 23, 2023

Conversation

ShGKme
Copy link
Contributor

@ShGKme ShGKme commented Oct 21, 2023 •
edited
Loading

A part of https://github.com/nextcloud/andy/issues/5

Error: The element label must not appear as a descendant of the label element.

  • Remove incorrect <label> wrapper
  • Use <NcTextField>'s icon feature instead of separated icon
  • Use recommended 20px icon size on all icons
  • Use CSS variables for spacing
  • Remove right margin from buttons
  • Remove aria-label-s. Buttons with texts should not have aria-label that is different from the button text.
Before After
image image

@ShGKme ShGKme added bug Something isn't working 3. to review Waiting for reviews accessibility labels Oct 21, 2023
@ShGKme ShGKme added this to the Nextcloud 28 milestone Oct 21, 2023
@ShGKme ShGKme self-assigned this Oct 21, 2023
susnux
susnux approved these changes Oct 21, 2023
View reviewed changes
Copy link
Contributor

@susnux susnux left a comment

Choose a reason for hiding this comment

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

clean and looks much better!

@susnux
Copy link
Contributor

susnux commented Oct 21, 2023

Just fix the node CI :)

@ShGKme
Copy link
Contributor Author

ShGKme commented Oct 21, 2023 •
edited
Loading

Just fix the node CI :)

Would you believe if I say, I did npm ci, npm run build, git status is clear and node is 20 with npm 9 🙈

ShGKme added 2 commits October 23, 2023 10:33
@ShGKme
fix: adjust new album form layout
f393de3 
- Remove incorrect `<label>`
- Set icon size 20px
- Fix buttons margin

Signed-off-by: Grigorii K. Shartsev <[email protected]>
@ShGKme
chore(assets): Re-compile assets
7562a46 
Signed-off-by: Grigorii K. Shartsev <[email protected]>
@ShGKme ShGKme force-pushed the fix/new-album-layout branch from 6393bd6 to 7562a46 Compare October 23, 2023 08:35
@ShGKme
Copy link
Contributor Author

ShGKme commented Oct 23, 2023

Just rebased and re-compiled

@ShGKme ShGKme merged commit c76a8bd into master Oct 23, 2023
23 checks passed
@ShGKme ShGKme deleted the fix/new-album-layout branch October 23, 2023 09:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JuliaKirschenheuter JuliaKirschenheuter JuliaKirschenheuter approved these changes

@susnux susnux susnux approved these changes

@artonge artonge Awaiting requested review from artonge

@Pytal Pytal Awaiting requested review from Pytal

@szaimen szaimen Awaiting requested review from szaimen

Assignees

@ShGKme ShGKme

Labels
3. to review Waiting for reviews accessibility bug Something isn't working
Projects
None yet
Milestone
Nextcloud 28
Development

Successfully merging this pull request may close these issues.
3 participants
@ShGKme @susnux @JuliaKirschenheuter