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

Add a new LabelPosition of inside to InputControl #55963

Open
andrewhayward opened this issue Nov 8, 2023 · 8 comments
Open

Add a new LabelPosition of inside to InputControl #55963

andrewhayward opened this issue Nov 8, 2023 · 8 comments
Assignees
Labels
[Feature] Data Views Work surrounding upgrading and evolving views in the site editor and beyond [Package] Components /packages/components [Status] In discussion Used to indicate that an issue is in the process of being discussed [Type] Enhancement A suggestion for improvement.

Comments

@andrewhayward
Copy link
Contributor

What problem does this address?

A use-case has emerged for InputControl to put the label inside of the control wrapper.

A label and input, all contained within a single border

This is not currently possible to do properly, and as such the consumer is using the prefix prop instead. This means that the input does not have an accessible name, and that part of the field is not clickable.

What is your proposed solution?

A new LabelPosition of inside should be added to InputControl, to allow consumers to put the label inside of the field, and not misuse the prefix option. This will better ensure that inputs are appropriately named.

@andrewhayward andrewhayward added [Type] Enhancement A suggestion for improvement. [Package] Components /packages/components [Feature] Data Views Work surrounding upgrading and evolving views in the site editor and beyond labels Nov 8, 2023
andrewhayward pushed a commit that referenced this issue Nov 8, 2023
This change adds a new `LabelPosition` value of `inside` to `InputControl`, allowing consumers to render the label visually inside the field.

Resolves #55963
@chad1008
Copy link
Contributor

chad1008 commented Nov 8, 2023

A new label position inside label LabelPosition sounds like a good way to satisfy this use-case to me. When trying to think of other approaches to fill this need, I can't come up with anything I don't actively dislike. So I'd vote for your suggestion!

@ciampo
Copy link
Contributor

ciampo commented Nov 9, 2023

Thank you for working on this!

A use-case has emerged for InputControl to put the label inside of the control wrapper.

Is this related to the work on Data View: searching and filtering?

For that specific scenario, do you think that adding a new label position for input control is the optimal solution, or could also evaluate alternatives? I'm mainly asking because I want to make sure that we're making the best decision here.

Before going ahead, it's probably a good idea to get some feedback from @WordPress/gutenberg-design too

@andrewhayward
Copy link
Contributor Author

Is this related to the work on Data View: searching and filtering?

Yes, this is for the data views filtering work...

A data views list, with filters above, showing combined labels and inputs.

For that specific scenario, do you think that adding a new label position for input control is the optimal solution, or could also evaluate alternatives? I'm mainly asking because I want to make sure that we're making the best decision here.

I considered a few options:

  1. An entirely new component is built for this purpose (within edit-site, alongside the data views work), giving the project the flexibility it needs to diverge
  2. The InputControl API is modified to allow prefixes/suffixes to access the generated ID, so that proper labels can be inserted into those slots
  3. Pass in an ID, and use a <label> in the prefix slot
  4. Add a new label position, but otherwise keep things as they are

I discounted the first option as being a lot of work that would mostly duplicate existing functionality, for relatively little gain. Equally, I discounted the idea of changing how prefixes/suffixes work, because I couldn't think of any obvious use-cases beyond labelling to make the work worthwhile.

The third option of providing an ID in combination with a prefixed <label> is definitely viable, but I ultimately opted for a new label position to better enforce field labelling.

Given the somewhat specific use-case here, I'm happy to simply pass in an ID and a <label> if that is preferred, though some minor modification to the prefix/suffix CSS in InputControl would be needed to allow full-height <label> elements within the field border.

@ciampo
Copy link
Contributor

ciampo commented Nov 9, 2023

Thank you for the context. I agree with the approach that you proposed, and I'm just going to add a few nuances:

  • Design-wise, it looks like the label styles may need tweaks (especially around its typography) to make the label "blend in" with the input's value. It would be good to understand whether those changes should be applied directly to InputControl, or if they should be specific to SelectControl
  • Also noting that Data view: Search & Filtering #55100 mentions the usage of SelectControl as a temporary solution while we work on more complex UI (dropdown + combobox + multiple selection). So, while I agree that this issue will improve the situation (also make InputControl-based components more flexible), it's good to know that we'll likely have to rewrite this part of the filter UI at some point

@andrewhayward
Copy link
Contributor Author

If the current situation is short-term, and the the plan is to rebuild the filters entirely (option one, above), then it might actually be better to leave InputControl alone entirely, and just manually associate a label with the input (option three), or provide a duplicated invisible value.

This will mean fewer moving parts, and no potential issues for breaking InputControl elsewhere.

andrewhayward pushed a commit that referenced this issue Nov 9, 2023
All user inputs should have an accessible name, ideally provided by a visible label. This patch ensures that data views "in" filters semantically associate the visible label with the select input, ensuring that the input has an accessible name.

Resolves #55963
Closes #55977
@oandregal
Copy link
Member

👋 I just wanted to share that the SelectControl was used to bootstrap the filters API: I plan to update the filters UI with their own custom control (details in the issue Marko shared). Presumably this work is going to be based on the dropdown component, the existing or future, I don't know. I still need to check out the current status of that work and how can I help there while building the filters UI. Hope this helps.

@andrewhayward andrewhayward added [Status] In discussion Used to indicate that an issue is in the process of being discussed and removed [Status] In Progress Tracking issues with work in progress labels Nov 10, 2023
@ntsekouras
Copy link
Contributor

ntsekouras commented Nov 28, 2023

Thanks for the issue! @oandregal is this issue still relevant with the updates you've made in filters? Or @andrewhayward has this been resolved here: #56001?

@ciampo
Copy link
Contributor

ciampo commented Nov 29, 2023

I think that the immediate need for the Data Views project has already been addressed with #56001 , although this issue can still stay open as a low-priority general improvement for the InputControl component

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Data Views Work surrounding upgrading and evolving views in the site editor and beyond [Package] Components /packages/components [Status] In discussion Used to indicate that an issue is in the process of being discussed [Type] Enhancement A suggestion for improvement.
Projects
Status: Inbox (needs triage) 📬
Development

Successfully merging a pull request may close this issue.

5 participants