fix: Display all options for CollectionPreferences content display

[youdz]

Description

Currently in the CollectionPreferences, if the preferences.contentDisplay prop includes 3 values but the contentDisplayPreference lists 50 options, we only display the 3 options. This seems like a clear bug, as the list of full options with label should probably be the source of truth on which ones to allow the customer to toggle and order.

This PR ensures options present in contentDisplayPreference but not in preferences.contentDisplay are displayed last and made visible: false. Given that this is a cheap bug fix, I figured I could open a PR and discuss directly here, and if I missed something and it is expected behavior then I lost very little time.

Note that in order to ensure the "missing" options can be toggled and reordered, the preferences.contentDisplay value needs to be updated. A common solution for this is to use an effect, but I believe this is an anti-pattern that results in effect hell. Instead, I decided to only add the missing options to the value right before calling onChange, so this is easily traceable and debuggable.

Related links, issue #, if available: None

How has this been tested?

Tested using the longOptionsList example in the CollectionPreferences demo page passing

contentDisplay: [
  { id: 'id_3', visible: true },
  { id: 'id_1', visible: true },
]

Before

After

I also ran unit tests to ensure backward compatibility, and added five new tests for the expected behavior as it's very easy to miss. The fact that no test failed seems to confirm the current behavior is not intended.

Review checklist

The following items are to be evaluated by the author(s) and the reviewer(s).

Correctness

• Changes include appropriate documentation updates.
• Changes are backward-compatible if not indicated, see CONTRIBUTING.md.
• Changes do not include unsupported browser features, see CONTRIBUTING.md.
• Changes were manually tested for accessibility, see accessibility guidelines.

Security

• If the code handles URLs: all URLs are validated through the checkSafeUrl function.

Testing

• Changes are covered with new/existing unit tests? ✅
• Changes are covered with new/existing integration tests? ✅

---

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[youdz]

I just realized I missed an update, the non-value options are not editable. My bad, I'll update the PR.

EDIT: Updated, the appended options can be toggled and reordered correctly. Live announcements are also correct.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.20%. Comparing base (a0670ee) to head (4c4e416).
Report is 7 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2807      +/-   ##
==========================================
- Coverage   96.20%   96.20%   -0.01%     
==========================================
  Files         761      761              
  Lines       21447    21451       +4     
  Branches     6940     6941       +1     
==========================================
+ Hits        20633    20636       +3     
- Misses        806      807       +1     
  Partials        8        8              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[gethinwebster]

I'm not sure about this pattern of having to call this addMissingOptionsInValue function in multiple places before calling onChange, could we instead use sortedOptions directly in place of value throughout this component? Or create a function that wraps onChange and does the addMissing in just one place?

[youdz]

Good call, I was deep in the weeds when I wrote this and reading it again, it does look weird. I'll fix that and submit a new revision right now.

EDIT: We end up with the trimming to { id, visible } being inline, but I think that's fair as one of the two places it's used does it directly baked into the toggle logic.

[youdz]

New revision is rebased on latest main, and addresses @gethinwebster's comment.