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

Edit header revamp #1233

Merged
merged 10 commits into from Aug 15, 2023
Merged

Edit header revamp #1233

merged 10 commits into from Aug 15, 2023

Conversation

yohanboniface
Copy link
Member

@yohanboniface yohanboniface commented Jul 25, 2023

cf #609

image

image

container
)
cancel.href = '#'
cancel.title = L._('Cancel edits')
cancel.textContent = L._('Cancel')
cancel.textContent = `${L._('Cancel all')} (Ctrl+Z)`
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm more inclined to standardize the keyboard shortcuts between brackets [ctrl+z], not sure if the <kbd> tag would help assistive technologies.

What about macOS devices? 😈 (more accustomed to cmd+z)

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm more inclined to standardize the keyboard shortcuts between brackets [ctrl+z]

Should I do that in a separate PR ? AFAIK we use parenthesis everywhere ATM.

not sure if the tag would help assistive technologies

Ah, could be nice! But now those are inside the title attribute, so that would need dealing with our own tooltips I guess.

@yohanboniface
Copy link
Member Author

"Mobile" mode:

image

image

There is still the "Disable editing" that does not fit when screen get smaller, but this is already the case, so I'd say we can deal with it later (need an icon, or at least a new wording, cf #556 )

@yohanboniface yohanboniface merged commit 5765daa into master Aug 15, 2023
@yohanboniface yohanboniface deleted the edit-header-revamp branch August 15, 2023 14:36
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.

None yet

2 participants