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.

Sign up for GitHub

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

Jump to bottom

Add select options on the fly keeping/updating id's #6378

Closed
1 task done
ritxi opened this issue Oct 19, 2023 · 8 comments · Fixed by #6526
Closed
1 task done

Add select options on the fly keeping/updating id's #6378

ritxi opened this issue Oct 19, 2023 · 8 comments · Fixed by #6526
Labels
Bug On Backlog released select Squad SBC Payroll

Comments

@ritxi
Copy link

ritxi commented Oct 19, 2023

Current behaviour

When an option is selected, if the list options changes, that refreshes the internally used list items changing their ids.

Expected behaviour

When adding options to a select I expect to keep previous options with same internal id or updating to the new id

CodeSandbox or Storybook URL

https://codesandbox.io/s/select-with-on-the-fly-all-option-rw4vl9?file=/src/index.js

JIRA Ticket (Sage Only)

SKPU-10261

Suggested Solution

When a select list of options change if the selected value is still on the list that should be updating the internal id reference to keep accessibility compliance.

Carbon Version

121.0.2

Design Tokens Version

4.25.0

What browsers are you seeing the problem on?

Chrome

What Operating System are you seeing the problem on?

MacOS

Anything else we should know?

No response

Confidentiality

  • I confirm there is no confidential or commercially sensitive information included.
@ritxi ritxi added Bug triage Triage Required labels Oct 19, 2023
@nicktitchmarsh
Copy link
Contributor

FE-6250

@nicktitchmarsh
Copy link
Contributor

@ritxi, can you give some context about why these options need to be dynamically populated and can't be shown on initial render?

@Parsium
Copy link
Contributor

Parsium commented Nov 28, 2023

Hi @ritxi 👋🏼 Would you be able to provide some more context regarding your use case?

@sianford
Copy link
Contributor

sianford commented Jan 3, 2024

Message sent in Slack to @ritxi requesting more context about their requirements.

@ritxi
Copy link
Author

ritxi commented Jan 9, 2024

Hi guys! This is still an accessibility issue. To reproduce just choose one option(A, B or C) and run axedev tools and you'll see the following error:
Captura de pantalla 2024-01-09 a les 8 38 17

@nicktitchmarsh
Copy link
Contributor

nicktitchmarsh commented Jan 9, 2024 •
edited
Loading

Hi @ritxi there is a simple workaround until we fix the ID persistence issue

id={opt}

Adding the IDs to each option during your render will ensure they always have the correct ID. I've added a ticket below for us to make this work without any further code. Also as a side note, would you not just want the All option to always be there?

https://codesandbox.io/p/sandbox/select-with-on-the-fly-all-option-forked-74ypdk?file=%2Fsrc%2Findex.js%3A27%2C29

@ritxi
Copy link
Author

ritxi commented Jan 10, 2024

Hi @ritxi there is a simple workaround until we fix the ID persistence issue

id={opt}

Adding the IDs to each option during your render will ensure they always have the correct ID. I've added a ticket below for us to make this work without any further code. Also as a side note, would you not just want the All option to always be there?

https://codesandbox.io/p/sandbox/select-with-on-the-fly-all-option-forked-74ypdk?file=%2Fsrc%2Findex.js%3A27%2C29

Thanks @nicktitchmarsh, but showing all option after some option is selected was part of the definition.

robinzigmond added a commit that referenced this issue Jan 17, 2024
@robinzigmond
fix(select): maintain option ids according to key when children count…
987e210 
… changes

Currently Select assigns a random guid as ID to each of its option children if no ids are provided
explicitly, and regenerates this list of ids whenever the number of children changes. This results
in unexpected behaviour when a new option is dynamically added to the start of the list on
selection, as then the ids are regenerated and a critical axe error results from the
aria-activedescendant property no longer referencing a valid child ID. To fix this, guids are now
generated in Option itself (rather than SelectList as before) when the prop is not provided, and it
is up to consumer to set keys on the list of option children to ensure individual child components
maintain their identity correctly across renders even as children are added or removed.

fix #6378
@robinzigmond robinzigmond mentioned this issue Jan 17, 2024
Merged
15 tasks
robinzigmond added a commit that referenced this issue Jan 22, 2024
@robinzigmond
fix(select): maintain option ids according to key when children count…
84ca2da 
… changes

Currently Select assigns a random guid as ID to each of its option children if no ids are provided
explicitly, and regenerates this list of ids whenever the number of children changes. This results
in unexpected behaviour when a new option is dynamically added to the start of the list on
selection, as then the ids are regenerated and a critical axe error results from the
aria-activedescendant property no longer referencing a valid child ID. To fix this, guids are now
generated in Option itself (rather than SelectList as before) when the prop is not provided, and it
is up to consumer to set keys on the list of option children to ensure individual child components
maintain their identity correctly across renders even as children are added or removed.

fix #6378
robinzigmond added a commit that referenced this issue Feb 7, 2024
@robinzigmond
fix(select): maintain option ids according to key when children count…
144583b 
… changes

Currently Select assigns a random guid as ID to each of its option children if no ids are provided
explicitly, and regenerates this list of ids whenever the number of children changes. This results
in unexpected behaviour when a new option is dynamically added to the start of the list on
selection, as then the ids are regenerated and a critical axe error results from the
aria-activedescendant property no longer referencing a valid child ID. To fix this, guids are now
generated in Option itself (rather than SelectList as before) when the prop is not provided, and it
is up to consumer to set keys on the list of option children to ensure individual child components
maintain their identity correctly across renders even as children are added or removed.

fix #6378
carbonci pushed a commit that referenced this issue Feb 7, 2024
@semantic-release-bot @carbonci
chore(release): 125.11.1
d32a42c 
### [125.11.1](v125.11.0...v125.11.1) (2024-02-07)

### Bug Fixes

* **select:** maintain option ids according to key when children count changes ([144583b](144583b)), closes [#6378](#6378)
@carbonci
Copy link
Collaborator

carbonci commented Feb 7, 2024

🎉 This issue has been resolved in version 125.11.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug On Backlog released select Squad SBC Payroll
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(select): maintain option ids according to key when children count changes Sage/carbon
5 participants
@ritxi @sianford @Parsium @carbonci @nicktitchmarsh