feat: icon-only Export and Share in the editor top bar - #368
Merged
Conversation
Export and Share showed an icon and a text label. Both are now icon only, which frees about 104px in the right cluster of the top bar. The removal exposed a row that did not hold together. Comments was already a borderless ghost button. Export, Share and the "..." button were outlined, and "..." rendered 6px wider than the others, because it wrapped a plain span instead of the Button icon slot. All four are now ghost icon buttons of one size, so the row reads as one set. The visible text was the accessible name. Each button now sets `label`, which frappe-ui Button renders as aria-label, and `tooltip`. An icon with neither is unidentifiable. The E2E specs find these buttons by role and name, so those specs needed no change. This also restores an accessible name that was never applied. A plain `aria-label` attribute on frappe-ui Button is discarded. Button spreads the inherited attributes, then writes `'aria-label': props.label` over them, so an unset `label` prop overwrites the attribute with undefined. The "..." and Comments buttons both used that attribute, and both reported no accessible name at all. This is one case of #176. Measured live in the editor at 1280, 1366, 1440, 1512, 1600, 1680 and 1920: the cluster content drops from 246px to 142px. Closes #229 Co-authored-by: Vibhav Katre <vibhav@frappe.io> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Export and Share showed an icon and a text label. Both are now icon only.
What changed
All four controls in the top bar right cluster are now ghost icon buttons of one
size, so the row reads as one set:
Removing the text exposed a row that did not hold together. Comments was already
borderless while the other three were outlined, and "..." rendered 6px wider
because it wrapped a plain span instead of the Button icon slot.
Accessible names
The visible text was the accessible name. Each button now sets
label, whichfrappe-ui Button renders as aria-label, and
tooltip. An icon with neither isunidentifiable.
This also restores an accessible name that was never applied. A plain
aria-labelattribute on frappe-ui Button is discarded: Button spreads theinherited attributes, then writes
'aria-label': props.labelover them, so anunset
labelprop overwrites the attribute withundefined. The "..." andComments buttons both used that attribute, and both reported no accessible name
at all. This is one case of #176.
Verification
Built and driven live in the editor at 1280, 1366, 1440, 1512, 1600, 1680 and
1920 (1280 is the supported minimum). The cluster content drops from 246px to
142px. Each button keeps its accessible name, gains a tooltip on hover, and
still opens what it opened before: the export menu, the overflow menu, the
sharing dialog and the comments panel.
yarn test— 858 pass, including 18 new checks innavbarActions.test.jsyarn build— cleanThe E2E suite has two pre-existing failures in
sharing.spec.js, on changing amember level and on removing a member. They reproduce identically on unmodified
mainin the same environment, and CI is green onmain, so they are local tothis bench. They match the root cause described in #194.
Closes #229