Problem/Motivation

Found in #3371005: Toolbar doesn't indicate active menu trail for pages not included in Toolbar
The breadcrumb is broken under the Appearance tab and seems to be working for settings but not update.

For Settings

For Update

Steps to reproduce

  1. Navigate to Appearance
  2. Try clicking on Update and Settings

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

CommentFileSizeAuthor
#5 after_fix.png122.24 KBomkar.podey
Screenshot 2023-07-20 at 5.35.57 PM.png63.17 KBomkar.podey
Screenshot 2023-07-20 at 5.35.50 PM.png82.13 KBomkar.podey

Issue fork drupal-3375850

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

omkar.podey created an issue. See original summary.

vishwenderbaloria’s picture

vishwenderbaloria commented

Checked on other version of drupal, the functionality is the same. What exactly is the requirement here ? Please clearify.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented

Since "Update" is a tab under "Appearance", "Appearance" should be listed in the breadcrumb. Basically the breadcrumb on this page should be the same as its on the "Settings" page.

omkar.podey opened merge request !4537

omkar.podey’s picture

omkar.podey commented
Issue summary: View changes
StatusFileSize
new after_fix.png122.24 KB

After the one line fix

omkar.podey’s picture

omkar.podey commented
Status: Active » Needs review

I don't think this small of a change requires testing, I also looked for /admin/theme/update and there seems to be only one instance so i think it should be fine.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community
Issue tags: +Needs Review Queue Initiative

Confirmed the breadcrumb issue on /admin/appearance/update
Applied MR 4537
Breadcrumbs on Update and Setting pages now match.

Also agree this is a small change that a test case would be overkill.

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Reviewed & tested by the community » Needs work

Posted comment on the MR

omkar.podey’s picture

omkar.podey commented
Assigned: Unassigned » omkar.podey
omkar.podey’s picture

omkar.podey commented
Assigned: omkar.podey » Unassigned
Status: Needs work » Needs review
narendrar’s picture

narendrar
He/Him
Primary language English
Location Jaipur, India
commented
Issue tags: +Needs change record

May be a CR for updated path is required.

tedbow’s picture

tedbow
he/him
Primary language English
Location Ithaca, NY, USA
commented
Component: base system » update.module
Status: Needs review » Needs work

Just changing the component because this is all in the Update module. Left 1 comment on the MR, not sure if it is important. Needs work for the deprecation version number

omkar.podey’s picture

omkar.podey commented
Issue tags: -Needs change record
omkar.podey’s picture

omkar.podey commented
Status: Needs work » Needs review

hooroomoo made their first commit to this issue’s fork.

hooroomoo’s picture

hooroomoo
they/them
Location nyc🗽
commented
Status: Needs review » Reviewed & tested by the community

Confirmed that going to admin/theme/update is a 301 response and the breadcrumb is visible on admin/appearance/update.

  • lauriii committed f4744069 on 11.x 
    Issue #3375850 by omkar.podey, hooroomoo, smustgrave, narendraR, tedbow...

lauriii closed merge request !4537

lauriii’s picture

lauriii
he/him
Primary language Finnish
Location Finland
commented
Status: Reviewed & tested by the community » Fixed

Committed f474406 and pushed to 11.x. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.