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

Theme selector with previous dark theme colors #5215

Closed
fcoveram opened this issue Nov 29, 2024 · 4 comments · Fixed by #5216
Closed

Theme selector with previous dark theme colors #5215

fcoveram opened this issue Nov 29, 2024 · 4 comments · Fixed by #5216
Assignees
Labels
🕹 aspect: interface Concerns end-users' experience with the software 🛠 goal: fix Bug fix 🟩 priority: low Low priority and doesn't need to be rushed 🧱 stack: frontend Related to the Nuxt frontend
Milestone

Comments

@fcoveram
Copy link
Contributor

Description

On staging, I noticed that after changing from system (default) to dark, the component kept showing the previous color scheme (dark pink shade).

Reproduction

mode.selector.recording.mp4
  1. Go to staging.openverse.org
  2. On homepage's footer, click on the theme select
  3. Select "Dark"
  4. Click out of the component to unfocus
  5. See the color mismatch
@fcoveram fcoveram added 🛠 goal: fix Bug fix 🟧 priority: high Stalls work on the project or its dependents labels Nov 29, 2024
@fcoveram fcoveram added this to the Dark Mode milestone Nov 29, 2024
@openverse-bot openverse-bot moved this to 📋 Backlog in Openverse Backlog Nov 29, 2024
@dhruvkb dhruvkb added 🟩 priority: low Low priority and doesn't need to be rushed 🕹 aspect: interface Concerns end-users' experience with the software 🧱 stack: frontend Related to the Nuxt frontend and removed 🟧 priority: high Stalls work on the project or its dependents labels Nov 29, 2024
@dhruvkb
Copy link
Member

dhruvkb commented Nov 29, 2024

This is specific to Safari so I have reduced the priority to low.

It is caused by the combination of two factors:

  • Safari doesn't support the click event on the <select> field.
  • Safari isn't storing cookies from staging.openverse.org or openverse.org.

The second factor should be extracted into a separate issue.

@dhruvkb dhruvkb self-assigned this Nov 29, 2024
@openverse-bot openverse-bot moved this from 📋 Backlog to 📅 To Do in Openverse Backlog Nov 29, 2024
@openverse-bot openverse-bot moved this from 📅 To Do to 🏗 In Progress in Openverse Backlog Nov 29, 2024
@obulat
Copy link
Contributor

obulat commented Dec 4, 2024

@dhruvkb, why doesn't Safari store cookies? That would be a big problem for rendering, wouldn't it?

@obulat
Copy link
Contributor

obulat commented Dec 4, 2024

One more problem I can see in Safari: the select becomes white inside on the light mode.
Image

I've also noticed it being pink in the dark (or system, I'm not sure now) mode in Arc, but I couldn't reliably reproduce it.
Image

@dhruvkb
Copy link
Member

dhruvkb commented Dec 4, 2024

I have not observed this problem at all in Arc or Firefox but it does occur in Safari. That's because Safari does not handle click events on <select>. I've pushed a fix for Safari in PR #5216 to use focus event instead of click, which also works on other browsers.

As for the cookies, after debugging this for an entire day, all I could tell is that Safari is very finicky about cookies (in the name of privacy) and does not store any cookies for Openverse.org at all. It's receiving the cookies in the response, parsing them too but then discarding them instead of storing. I need to try this again without extensions to be certain it's not one my extensions that's responsible for this behaviour.

@openverse-bot openverse-bot moved this from 🏗 In Progress to ✅ Done in Openverse Backlog Dec 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🕹 aspect: interface Concerns end-users' experience with the software 🛠 goal: fix Bug fix 🟩 priority: low Low priority and doesn't need to be rushed 🧱 stack: frontend Related to the Nuxt frontend
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants