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

Site Title: Extract Heading Level Dropdown from Heading block, re-use #24768

Closed
ockham opened this issue Aug 24, 2020 · 4 comments
Closed

Site Title: Extract Heading Level Dropdown from Heading block, re-use #24768

ockham opened this issue Aug 24, 2020 · 4 comments
Labels
[Package] Components /packages/components [Type] Code Quality Issues or PRs that relate to code quality

Comments

@ockham
Copy link
Contributor

ockham commented Aug 24, 2020

We have a component at packages/block-library/src/heading/heading-level-dropdown.js which seems to be very similar to the one used by Site Title. Probably we should extract the component from the Heading block and reuse it here.

(Per @david-szabo97 in #24758.)

@ockham ockham added the [Package] Components /packages/components label Aug 24, 2020
@ZebulanStanphill ZebulanStanphill added the [Type] Code Quality Issues or PRs that relate to code quality label Aug 25, 2020
@ZebulanStanphill
Copy link
Member

ZebulanStanphill commented Aug 25, 2020

It's worth noting the current a11y issues with the Heading block's implementation that should probably be resolved before using it for other blocks.

@Addison-Stavlo
Copy link
Contributor

From what I am gathering from that issue:

While the current implementation works , this issue is more about what users would expect

This seems more of a design concern than an accessibility blocker. We should be able to move forward with extracting this component for re-use, as making improvements on it in the future would then improve the flow everywhere it is used.

@ZebulanStanphill
Copy link
Member

@Addison-Stavlo That's a good point, and I would not be opposed to a PR to go ahead and merge the two implementations. However, there is one more thing I should point out:

The Heading block's level dropdown uses icons for its visible option labels. Normally, this would be less than ideal, but the icons in this case are almost just text. (H1, H2, etc.) In the case of the Site Title block, displaying the text as a Paragraph is an option, which as an icon would have to be represented either using the icon we already use for the Paragraph block, or else a "P". I don't think this is ideal, especially for users that aren't English-speaking.

I'm starting to wonder if perhaps the Site Title block's tag dropdown should be ported to the Heading block, rather than the other way around. The Site Title block's dropdown is clearly the better-implemented of the two.

@skorasaurus
Copy link
Member

This can be closed thanks to the work done in #46003

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Components /packages/components [Type] Code Quality Issues or PRs that relate to code quality
Projects
None yet
Development

No branches or pull requests

4 participants