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

[Bug]: Picker does not keep up with changes to icons in its menu items #5081

Open
1 task done
estollnitz opened this issue Feb 7, 2025 · 3 comments · May be fixed by #5088
Open
1 task done

[Bug]: Picker does not keep up with changes to icons in its menu items #5081

estollnitz opened this issue Feb 7, 2025 · 3 comments · May be fixed by #5088

Comments

@estollnitz
Copy link
Collaborator

Code of conduct

  • I agree to follow this project's code of conduct.

Impacted component(s)

Picker

Expected behavior

When there's a change to the src attribute of an sp-icon used within a menu item, the selected item displayed by the picker should update to reflect the modified icon.

Actual behavior

Picker displays a stale icon associated with the selected menu item when the icon has changed to a new src value. The menu items shown in the popover are correct; only the copy shown in the picker button itself is stale.

Screenshots

Image

What browsers are you seeing the problem in?

Firefox, Chrome, Safari, Microsoft Edge

How can we reproduce this issue?

See the sample code below. It displays repro steps that are easy to follow.

Sample code or abstract reproduction which illustrates the problem

https://studio.webcomponents.dev/edit/wF3pii8z9xOpdkyufNqu/src/index.ts?p=stories

Severity

SEV 3

Logs taken while reproducing problem

No response

@estollnitz estollnitz added bug Something isn't working needs jira ticket triage An issue needing triage labels Feb 7, 2025
@estollnitz
Copy link
Collaborator Author

I think the issue stems from MenuItem caching a clone of its icon the first time that Picker requests the itemChildren getter here: https://github.com/adobe/spectrum-web-components/blob/main/packages/menu/src/MenuItem.ts#L196. The result gets put into MenuItem's _itemChildren cache, and that cache is not invalidated when I update the src attribute of an sp-icon. The cache is only invalidated when the MutationController sees a change to characterData or childList or subtree and then calls breakItemChildrenCache here: https://github.com/adobe/spectrum-web-components/blob/main/packages/menu/src/MenuItem.ts#L218.

@estollnitz
Copy link
Collaborator Author

Seems like a relatively simple fix would be to include either attributes: true or (if you want to be more specific) attributeFilter: ['src'] in the config for the MutationController here: https://github.com/adobe/spectrum-web-components/blob/main/packages/menu/src/MenuItem.ts#L218.

@estollnitz
Copy link
Collaborator Author

I confirmed that calling the breakItemChildrenCache method of each MenuItem whenever I update its icon will indeed cause the Picker to display the correct icon. I wrote this hacky workaround to call the protected breakItemChildrenCache method:

    const menuItems = this.shadowRoot.querySelectorAll('sp-menu-item')
    for (const menuItem of menuItems) {
      (menuItem as any).breakItemChildrenCache();
    }

Of course, this is just a workaround — I should not have to call a protected method of a class I don't own.

@estollnitz estollnitz linked a pull request Feb 12, 2025 that will close this issue
14 tasks
@estollnitz estollnitz linked a pull request Feb 12, 2025 that will close this issue
14 tasks
@najikahalsema najikahalsema added WIP Component: Picker and removed triage An issue needing triage labels Feb 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants