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 Editor: "Site" menu item is active when editing a template #36821

Closed
jameskoster opened this issue Nov 24, 2021 · 10 comments · Fixed by #42807
Closed

Site Editor: "Site" menu item is active when editing a template #36821

jameskoster opened this issue Nov 24, 2021 · 10 comments · Fixed by #42807
Assignees
Labels
[Feature] Site Editor Related to the overarching Site Editor (formerly "full site editing") Needs Accessibility Feedback Need input from accessibility [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended

Comments

@jameskoster
Copy link
Contributor

Screenshot 2021-11-24 at 14 35 38

The "Site" menu item should not be active in this case.

@jameskoster jameskoster added [Type] Bug An existing feature does not function as intended [Feature] Site Editor Related to the overarching Site Editor (formerly "full site editing") labels Nov 24, 2021
@jameskoster jameskoster added this to 📥 To do in WordPress 5.9 Must-Haves via automation Nov 24, 2021
@Mamaduka
Copy link
Member

Can you give me more details/specs when this menu item should be active and when not?

@jameskoster
Copy link
Contributor Author

Sure. Essentially, the "Site" menu item should only be active when:

  • You click "Appearance > Editor" (this works currently)
  • You click the Site menu item itself (this also works)

In any other case the flow is template-centric (IE you are navigating through the Templates List), so it doesn't make sense to make the "Site" item active in those situations.

I appreciate this is a little tricky to pin down since template + site editing exist in the same package, but the split in the UX makes this feel quite important.

@kevin940726
Copy link
Member

The problem is that we do a redirection upon clicking on the "Site" link. There's essentially no page which is called "Site", we immediately redirect the user to the template editor (which is the frontpage template by default). Should we:

  • Highlight the "Site" link only when we're on the default(frontpage) template, or
  • Highlight the "Site" link whenever we're editing any template but not template parts,
  • Or anything else

Since we always do a full navigation, so we don't have the information of where we're coming from (whether we click on the "Site" link or not). Since this would likely be linked to the aria-current attribute in #36946, I guess it makes sense to have accessibility folks giving feedbacks on this as well. WDYT?

@kevin940726 kevin940726 added the Needs Accessibility Feedback Need input from accessibility label Nov 29, 2021
@jameskoster
Copy link
Contributor Author

None of these options seem very good to me. It would be very strange imo to see "Site" as the active menu item when editing the Page template via the templates list (assuming a static home page it set in this example).

Probably a dumb question but... logic exists somewhere to interpret the homepage settings in order to display the correct template / content when opening the site editor. Can we not use that same logic to determine when to set the active state on the Site menu item?

@Mamaduka
Copy link
Member

I think we can check if page?.path === siteUrl. Currently, this is only true for the home/index page.

@jameskoster
Copy link
Contributor Author

@Mamaduka would that same logic allow us to display the site name / url in the top bar when editing the homepage? (#36207)

@Mamaduka
Copy link
Member

Technically yes. I can create a proof of concept PR later this week.

@kevin940726
Copy link
Member

Probably a dumb question but... logic exists somewhere to interpret the homepage settings in order to display the correct template / content when opening the site editor. Can we not use that same logic to determine when to set the active state on the Site menu item?

Isn't this the same as my first proposal? Or am I missing something 🤔 ?

Highlight the "Site" link only when we're on the default(frontpage) template

@jameskoster
Copy link
Contributor Author

I don't think it's quite the same, but perhaps I am misinterpreting.

Highlight the "Site" link only when we're on the default(frontpage) template

In this case, if a static page were set as the front page, wouldn't this mean that the "Site" menu item would be active if you navigated to Templates > Page? That would be kind of confusing.

@Mamaduka
Copy link
Member

Mamaduka commented Jan 5, 2022

@jameskoster, I tried to make this work, but it requires more changes than I expected.

I'm going to remove this from the WP 5.9 board since it's not a block, and we're already in the RC phase.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Site Editor Related to the overarching Site Editor (formerly "full site editing") Needs Accessibility Feedback Need input from accessibility [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
No open projects
Development

Successfully merging a pull request may close this issue.

3 participants