feat(switch): add syncLevel for switch storage scopes#7222
feat(switch): add syncLevel for switch storage scopes#7222
Conversation
There was a problem hiding this comment.
Pull request overview
Adds a syncLevel option to Switch toggle persistence so switch state can be stored at different localStorage scopes (site/wiki/page) via a new data-sync-level attribute consumed by the SwitchButtons JS module.
Changes:
- Add
syncLevelprop to the LuaSwitchwidget and emit it asdata-sync-level. - Extend
javascript/commons/SwitchButtons.jsto readdata-sync-leveland build scope-specific localStorage keys (site/wiki/page).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| lua/wikis/commons/Widget/Switch.lua | Introduces syncLevel prop (default site) and outputs data-sync-level on the toggle element. |
| javascript/commons/SwitchButtons.js | Adds syncLevel to switchGroup state and updates localStorage key generation to respect site/wiki/page scope. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
lua/wikis/commons/Widget/Switch.lua
Outdated
| local VALID_SYNC_LEVELS = { | ||
| 'page', | ||
| 'wiki', | ||
| 'site', | ||
| } |
There was a problem hiding this comment.
| local VALID_SYNC_LEVELS = { | |
| 'page', | |
| 'wiki', | |
| 'site', | |
| } | |
| ---@enum ValidSyncLevels | |
| local VALID_SYNC_LEVELS = { | |
| page = 'page', | |
| wiki = 'wiki', | |
| site = 'site', | |
| } |
imo SwitchSyncLevel would be a better name
There was a problem hiding this comment.
Changed to SwitchSyncLevel as pascal case. Although there seems to be both naming conventions used for local vars, not sure which is better. pascal or upper snake
hjpalpha
left a comment
hjpalpha
left a comment
There was a problem hiding this comment.
lgtm modulo the comment from eboy
| buildLocalStorageKey: function ( groupName, syncLevel ) { | ||
| const base = 'LiquipediaSwitchButtons'; | ||
|
|
||
| if ( syncLevel === 'site' ) { |
|
|
||
| buildLocalStorageKey: function () { | ||
| buildLocalStorageKey: function ( groupName, syncLevel ) { | ||
| const base = 'LiquipediaSwitchButtons'; |
There was a problem hiding this comment.
move this to a high level so it's more visible
Summary
Add
syncLevelproperty to Switch widget allowing to control the storage scope of toggle states:site(default): shared across entire Liquipediawiki: shared within a single wikipage: unique per pageHow did you test this change?
dev + devtools
How to test
Toggle the swiches on a pages, check if switches on other pages are updated after refresh. Also check if the localStorage values are updated as intended.
https://liquipedia.net/rainbowsix/User:Eetwalt
https://liquipedia.net/leagueoflegends/User:Eetwalt
https://liquipedia.net/dota2/User:Eetwalt