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

fix(a11y): contrast for active menubar buttons #5098

Merged
merged 4 commits into from
Dec 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 19 additions & 16 deletions cypress/e2e/workspace.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,13 @@ describe('Workspace', function() {
]
cy.getContent().click()
buttons.forEach(([button, tag]) => testButtonUnselected(button, tag))
cy.getContent().type('Format me{selectall}')
// format is gone when text is gone
cy.getContent().type('Format me')
cy.getContent().find('s')
.should('not.exist')
cy.getContent()
.should('contain', 'Format me')
cy.getContent().type('{selectall}')
buttons.forEach(([button, tag]) => testButton(button, tag, 'Format me'))
})

Expand Down Expand Up @@ -303,13 +309,15 @@ const openSidebar = filename => {
* @param {string} content Content expected in the element.
*/
function testButton(button, tag, content) {
cy.getMenuEntry(button).click()
cy.getMenuEntry(button).should('have.class', 'is-active')
cy.getMenuEntry(button)
.should('not.have.class', 'is-active')
.click()
cy.getContent()
.find(`${tag}`)
.should('contain', content)
cy.getMenuEntry(button).click()
cy.getMenuEntry(button).should('not.have.class', 'is-active')
cy.getMenuEntry(button)
.should('have.class', 'is-active')
.click()
}

/**
Expand All @@ -318,16 +326,11 @@ function testButton(button, tag, content) {
* @param {string} tag Html tag expected to be toggled.
*/
function testButtonUnselected(button, tag) {
cy.getMenuEntry(button).click()
cy.getMenuEntry(button).should('have.class', 'is-active')
cy.getContent().type('Format me{selectall}')
cy.getMenuEntry(button)
.should('not.have.class', 'is-active')
.click()
cy.getContent().type('Format me')
cy.getContent().find(`${tag}`)
.should('contain', 'Format me').type('{del}')
cy.getMenuEntry(button).click()
cy.getMenuEntry(button).should('have.class', 'is-active').click()
cy.getMenuEntry(button).should('not.have.class', 'is-active')
cy.getContent().type('Format me{selectall}')
cy.getMenuEntry(button).find(`${tag}`)
.should('not.exist')
cy.getContent().type('{del}')
.should('contain', 'Format me')
cy.getContent().type('{selectall}{del}')
}
30 changes: 5 additions & 25 deletions src/components/Menu/ActionEntry.scss
Original file line number Diff line number Diff line change
@@ -1,20 +1,10 @@
%text__is-active-item-btn {
opacity: 1;
background-color: var(--color-primary-element-light);
border-radius: 50%;
.material-design-icon > svg {
fill: var(--color-primary-element);
}
}

.text-menubar, .v-popper__inner {
button.entry-action__button {
height: 44px;
margin: 0;
border: 0;
// opacity: 0.5;
position: relative;
color: var(--color-main-text);
background-color: transparent;
vertical-align: top;
box-shadow: none;
Expand Down Expand Up @@ -49,25 +39,15 @@
box-shadow: var(--color-primary-element);
}

&.is-active {
@extend %text__is-active-item-btn;
}
}

.entry-action.is-active:not(.entry-action-item) {
@extend %text__is-active-item-btn;
}

.entry-action.entry-action-item {
&.is-active {
background-color: var(--color-primary-element-light);
border-radius: var(--border-radius-large);
}
.button-vue {
margin-right: 2px;
}

.button-vue {
svg {
fill: var(--color-main-text);
@media only screen and (max-width: 767px) {
.button-vue {
margin-right: 0;
}
}

Expand Down
3 changes: 3 additions & 0 deletions src/components/Menu/ActionList.vue
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@
v-bind="state"
:container="menuIDSelector"
:aria-label="actionEntry.label"
:aria-pressed="state.active"
:type="state.active ? 'primary': 'tertiary'"
:aria-activedescendant="currentChild ? `${$menuID}-child-${currentChild.key}` : null"
:force-menu="true"
:name="actionEntry.label"
Expand All @@ -39,6 +41,7 @@
<ActionSingle v-for="child in children"
:id="`${$menuID}-child-${child.key}`"
:key="`child-${child.key}`"
:active="currentChild?.key === child.key"
is-item
:action-entry="child"
v-on="$listeners"
Expand Down
5 changes: 4 additions & 1 deletion src/components/Menu/ActionSingle.vue
Original file line number Diff line number Diff line change
Expand Up @@ -128,11 +128,14 @@ export default {
class: classes,
attrs: {
title,
type: 'tertiary',
type: attrs.active ? 'primary' : 'tertiary',
role: 'menuitem',
'data-text-action-entry': actionEntry.key,
...attrs,
},
props: isItem
? { pressed: attrs.active }
: {},
on: {
...$listeners,
click: runAction,
Expand Down
5 changes: 4 additions & 1 deletion src/components/Menu/MenuBar.vue
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ import { MENU_ID } from './MenuBar.provider.js'
import { DotsHorizontal, TranslateVariant } from '../icons.js'
import {
useEditorMixin,
useIsMobileMixin,
useIsRichEditorMixin,
useIsRichWorkspaceMixin,
} from '../Editor.provider.js'
Expand All @@ -119,6 +120,7 @@ export default {
extends: ToolBarLogic,
mixins: [
useEditorMixin,
useIsMobileMixin,
useIsRichEditorMixin,
useIsRichWorkspaceMixin,
],
Expand Down Expand Up @@ -204,7 +206,8 @@ export default {

// leave some buffer - this is necessary so the bar does not wrap during resizing
const spaceToFill = width - 4
const slots = Math.floor(spaceToFill / 44)
const spacePerSlot = this.$isMobile ? 44 : 46
const slots = Math.floor(spaceToFill / spacePerSlot)

// Leave one slot empty for the three dot menu
this.iconsLimit = slots - 1
Expand Down