fides icon indicating copy to clipboard operation
fides copied to clipboard

Updates Language-selector to be react-driven, ensuring it closes after selection

Open eastandwestwind opened this issue 6 months ago • 6 comments

Closes https://ethyca.atlassian.net/browse/ENG-721

Description Of Changes

Updates Language-selector to automatically close after language selection

https://www.loom.com/share/c610e9c428494874b543a9751cb8c370

Code Changes

  • Update open/close state to be react-driven
  • Update CSS accordingly

Steps to Confirm

  1. Navigate to an experience with multiple languages
  2. For both the banner and modal components do the following:
  3. Hover over the language selector, confirm the language modal opens as before
  4. Make a selection, confirm it closes automatically
  5. Hover over it again, then stop hovering over it. Confirm it closes as before

Pre-Merge Checklist

  • [ ] Issue requirements met
  • [ ] All CI pipelines succeeded
  • [ ] CHANGELOG.md updated
    • [ ] Add a https://github.com/ethyca/fides/labels/db-migration label to the entry if your change includes a DB migration
    • [ ] Add a https://github.com/ethyca/fides/labels/high-risk label to the entry if your change includes a high-risk change (i.e. potential for performance impact or unexpected regression) that should be flagged
    • [ ] Updates unreleased work already in Changelog, no new entry necessary
  • Followup issues:
    • [ ] Followup issues created
    • [ ] No followup issues
  • Database migrations:
    • [ ] Ensure that your downrev is up to date with the latest revision on main
    • [ ] Ensure that your downgrade() migration is correct and works
      • [ ] If a downgrade migration is not possible for this change, please call this out in the PR description!
    • [ ] No migrations
  • Documentation:
    • [ ] Documentation complete, PR opened in fidesdocs
    • [ ] Documentation issue created in fidesdocs
    • [ ] If there are any new client scopes created as part of the pull request, remember to update public-facing documentation that references our scope registry
    • [ ] No documentation updates required

eastandwestwind avatar Jun 16 '25 15:06 eastandwestwind

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
fides-plus-nightly 🛑 Canceled (Inspect) Jul 2, 2025 6:07pm
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
fides-privacy-center ⬜️ Ignored (Inspect) Jul 2, 2025 6:07pm

vercel[bot] avatar Jun 16 '25 15:06 vercel[bot]

@eastandwestwind - I have tested this on mobile and verified it addresses the issue there - it shows and hides as intended. I'm less familiar with the web, but I did notice that it opens immediately on mouse over, and I'm not sure if that's the same as the behavior was before, but it works fine on web as well. So looks good in testing

Approving the PR (the merge conflict should be easy to resolve) - I did have one question after reviewing

thabofletcher avatar Jun 16 '25 21:06 thabofletcher

One more note here: This isn't working at all on a mobile sized screen for the TCF banner. Some of the suggestions above may help mitigate that but be sure to include that in your dev testing.

gilluminate avatar Jun 17 '25 16:06 gilluminate

One more note here: This isn't working at all on a mobile sized screen for the TCF banner. Some of the suggestions above may help mitigate that but be sure to include that in your dev testing.

Not sure if this is what you are seeing Jason, but I'm getting some inconsistent behavior here in my local environment. I had originally tested this and found it working, but it is sometimes not showing up for me in mobile Safari and in the SDK webview

You'll notice in the screenshot where it isn't working the privacy notice rows aren't loading either - and looking at the DOM when its broken, I found this wasn't an issue with the element being hidden, but missing altogether, so it might be an issue with the translations from the backend not loading as the dropdown won't load if there are no translations from the BE

Screenshot 2025-06-17 at 1 25 29 PM Screenshot 2025-06-17 at 1 26 02 PM

thabofletcher avatar Jun 17 '25 20:06 thabofletcher

Not sure if this is what you are seeing Jason

@thabofletcher these screenshots appear to be from the TCF Modal, I'm seeing issues on TCF Banner specifically, where the button doesn't open the language selector at all.

gilluminate avatar Jun 17 '25 20:06 gilluminate

Not sure if this is what you are seeing Jason

@thabofletcher these screenshots appear to be from the TCF Modal, I'm seeing issues on TCF Banner specifically, where the button doesn't open the language selector at all.

That's probably ok from a mobile SDK perspective since we use fides_disable_banner=true to skip the banner in banner_and_modal. But maybe something to discover and address just from an overall correct behavior standpoint?

thabofletcher avatar Jun 19 '25 23:06 thabofletcher

fides    Run #13055

Run Properties:  status check passed Passed #13055  •  git commit 1e58497d61: Updates Language-selector to be react-driven, ensuring it closes after selection...
Project fides
Branch Review main
Run status status check passed Passed #13055
Run duration 00m 54s
Commit git commit 1e58497d61: Updates Language-selector to be react-driven, ensuring it closes after selection...
Committer Catherine Smith
View all properties for this run ↗︎

Test results
Tests that failed  Failures 0
Tests that were flaky  Flaky 0
Tests that did not run due to a developer annotating a test with .skip  Pending 0
Tests that did not run due to a failure in a mocha hook  Skipped 0
Tests that passed  Passing 5
View all changes introduced in this branch ↗︎

cypress[bot] avatar Jul 02 '25 18:07 cypress[bot]