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 "selected" styling to active elements in the design selector. #65127

Closed
juanfra opened this issue Sep 6, 2024 · 11 comments · Fixed by #65917
Closed

Add "selected" styling to active elements in the design selector. #65127

juanfra opened this issue Sep 6, 2024 · 11 comments · Fixed by #65917
Labels
Good First Issue An issue that's suitable for someone looking to contribute for the first time Needs Design Feedback Needs general design feedback. [Type] Enhancement A suggestion for improvement.

Comments

@juanfra
Copy link
Member

juanfra commented Sep 6, 2024

What problem does this address?

While working on TT5, I noticed no visual indication of the design chosen for templates. It would be nice to know at a first glance which design is active.

Screen.Recording.2024-09-06.at.17.43.57.mov

To reproduce:

  1. Download Twenty Twenty-Five, or use a theme that comes with different designs for some templates.
  2. Locate that template in the site editor.
  3. Change the design, and see that there's no indication.

What is your proposed solution?

We could leverage what we already have. The :focus style for that same element already has a visual hint (border with the admin accent color). We know with the data-active-item which element is active.

The other option, which can also be in line with the design patterns we already have in place, could be replicating what we have for the "selected" state in the "Templates" page of the site editor.

Screen.Recording.2024-09-06.at.17.56.55.mov
@juanfra juanfra added [Type] Enhancement A suggestion for improvement. Good First Issue An issue that's suitable for someone looking to contribute for the first time Needs Design Feedback Needs general design feedback. labels Sep 6, 2024
@jasmussen
Copy link
Contributor

CC: @scruffian @MaggieCabrera , I think you may have worked on the inspector control. Thank you for looking.

@patil-vipul
Copy link

@juanfra @jasmussen I'll look into this and create a PR for the same.
Also, I feel like the design of "selected" state in the "Templates" page of the site editor can be used here as well, just without the checkbox.

@ciampo
Copy link
Contributor

ciampo commented Oct 8, 2024

Hey folks! Coming back to this issue after a first round of review on #65917 and wanted to add a clarification to this suggestion:

We know with the data-active-item which element is active

That is true! Although we won't be able to rely on the "active" item to style the "selected" item — there's a difference between the two:

  • active, in this context, refers specifically to the active composite item, ie. the item in the composite widget that has keyboard focus when the composite widget is being interacted with
  • selected is the selected pattern. Active and selected do not always coincide — ie. a user may select a pattern, and then navigate the list again using arrow keys, thus changing the active item without updating the selected item. Having said that, we should make sure that, when a new item gets selected, we update the active item to be the same as the selected item.

This scenario is similar to what happens in https://github.com/WordPress/gutenberg/blob/8a7df39e11758aade23c586b435830a5d35aa5f2/packages/dataviews/src/dataviews-layouts/list/index.tsx

I'm leaving this clarification because folks who are not knowledgeable about how composite widgets and the Composite component work may get confused.

@juanfra
Copy link
Member Author

juanfra commented Oct 8, 2024

Thanks for the clarification Marco! Indeed, the suggestion is to add styling over the "selected" element.

That is true! Although we won't be able to rely on the "active" item to style the "selected" item

Definitely! I referenced the data-active-item to show that we have some data available, but we agree that the style should be applied to the "selected" element.

@benazeer-ben
Copy link
Contributor

Hi @ciampo , @juanfra

Thanks for all the details and explanations that you have shared.

Now it is more clear regarding the scenario that we need to handle. So I will look into these points in details and work on some alternative solutions.

Just clarifying that, we need the selected pattern styling along with this, right?

Regards,
Benazeer

@juanfra
Copy link
Member Author

juanfra commented Oct 9, 2024

Thanks @benazeer-ben

Just clarifying that, we need the selected pattern styling along with this, right?

We need to highlight which design is selected, right.

@benazeer-ben
Copy link
Contributor

Hi @ciampo

I have updated the code as per your feedbacks and committed. So please have a look on that and share your thoughts on this.

I have also added separate explanations for each feedback that you have provided.

So now, we are highlighting which design is selected.

Regard,
Benazeer.

@ciampo
Copy link
Contributor

ciampo commented Oct 18, 2024

@WordPress/gutenberg-design , could you advise on the styles for the selected design (pattern) ? I have a draft proposal in #66185 but it definitely needs some design feedback

@jasmussen
Copy link
Contributor

Can we use the same active state as is designed for style variations here?

Image

@ciampo
Copy link
Contributor

ciampo commented Oct 18, 2024

Got it — so the standard "primary color" (blueberry) outline for focused items, and a black outline for selected.

Kapture.2024-10-18.at.15.10.24.mp4

Focus styles and hover styles should override selected styles.

@jasmussen
Copy link
Contributor

Yes. We can evolve the treatment if need be, for the moment the main point is they should be the same.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Good First Issue An issue that's suitable for someone looking to contribute for the first time Needs Design Feedback Needs general design feedback. [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants