Skip to content

fix(web): persisted store #18385

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
Merged
merged 10 commits into from
Jun 3, 2025
Merged

fix(web): persisted store #18385

merged 10 commits into from
Jun 3, 2025
+64 −40

Conversation

wuzihao051119
Copy link
Collaborator

@wuzihao051119 wuzihao051119 commented May 20, 2025 •
edited
Loading

Fix failure in persisting the app settings and wrong fix in PR #17284.
PR #17284 will cause invalid default locale setting, because default locale setting will be seen as en-US.

Update: fix #18453.

@alextran1502
Copy link
Contributor

What issue is it causing and how do you reproduce it?

@wuzihao051119
Copy link
Collaborator Author

wuzihao051119 commented May 20, 2025 •
edited
Loading

2025-05-20.160655.mp4

It happens on the non-en-US device.
And some setting switches are no response.

@wuzihao051119 wuzihao051119 mentioned this pull request May 22, 2025
Closed
4 tasks
@wuzihao051119 wuzihao051119 force-pushed the fix-persisted-store branch 2 times, most recently from c86276f to 836956f Compare May 22, 2025 11:56
@Mdkar Mdkar mentioned this pull request May 30, 2025
Open
4 tasks
@alextran1502
Copy link
Contributor

Hello, can you help fix the failed tests?

@wuzihao051119 wuzihao051119 mentioned this pull request Jun 2, 2025
Closed
4 tasks
@alextran1502
Copy link
Contributor

Preview env only works for PRs opened directly from the project, not on fork

@wuzihao051119 wuzihao051119 force-pushed the fix-persisted-store branch from 836956f to c2c7395 Compare June 2, 2025 19:11
@wuzihao051119 wuzihao051119 force-pushed the fix-persisted-store branch from c2c7395 to ae6bc1b Compare June 2, 2025 19:31
@wuzihao051119
Copy link
Collaborator Author

wuzihao051119 commented Jun 2, 2025 •
edited
Loading

Done.
A question is after moving to the new ui library, the key of translation of theme is renamed from toggle_theme to darkTheme.

  • Fix the translation of themeSelection button.
  • Fix locale switch cannot be persisted and test.
  • Fix alwaysLoadOriginalFile, playVideoThumbnailOnHover and loopVideo switch cannot be changed.

For who has encountered issues #17284 and #17270, you should clear blank locale value in localStorage.

@alextran1502
Copy link
Contributor

Can you only keep the changes to the en.json file and revert the changes from other locales?

@wuzihao051119
Copy link
Collaborator Author

wuzihao051119 commented Jun 3, 2025 •
edited
Loading

Everything is ok now.

danieldietzler
danieldietzler approved these changes Jun 3, 2025
View reviewed changes
Copy link
Member

@danieldietzler danieldietzler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I haven't tested it but this looks correct to me, thanks!

@alextran1502
Copy link
Contributor

If the locale key is blank, the website won't load

image

We need to handle this case becauses it happened previously

@wuzihao051119
Copy link
Collaborator Author

Okay, handle blank locale. It is so complex because too many things about locale need considering.

@wuzihao051119 wuzihao051119 force-pushed the fix-persisted-store branch from 35ac488 to f01dabb Compare June 3, 2025 15:43
alextran1502
alextran1502 approved these changes Jun 3, 2025
View reviewed changes
Copy link
Contributor

@alextran1502 alextran1502 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@alextran1502 alextran1502 enabled auto-merge (squash) June 3, 2025 15:54
@wuzihao051119 wuzihao051119 disabled auto-merge June 3, 2025 16:00
@wuzihao051119 wuzihao051119 enabled auto-merge (squash) June 3, 2025 16:04
@wuzihao051119 wuzihao051119 merged commit daf1bee into immich-app:main Jun 3, 2025
46 checks passed
@wuzihao051119 wuzihao051119 deleted the fix-persisted-store branch June 4, 2025 01:27
savely-krasovsky pushed a commit to savely-krasovsky/immich that referenced this pull request Jun 8, 2025
@wuzihao051119 @savely-krasovsky
fix(web): persisted store (immich-app#18385)
ea7e3b2 
* fix(web): persisted store

* fix: translation

* fix: test

* fix: test

* revert i18n changes

* fix blank locale
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alextran1502 alextran1502 alextran1502 approved these changes

@danieldietzler danieldietzler danieldietzler approved these changes

Assignees
No one assigned
Labels
changelog:bugfix 🖥️web
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

The "Play video thumbnail on hover" and "Loop videos" options in the settings are not working
4 participants
@wuzihao051119 @alextran1502 @danieldietzler @NicholasFlamy