Bug 2037736 - Added a store to the settings and implemented storeProvider for persisted state#255
Bug 2037736 - Added a store to the settings and implemented storeProvider for persisted state#255fmasalha wants to merge 1 commit into
Conversation
|
View this pull request in Lando to land it once approved. |
…ider for persisted state
fmasalha
force-pushed
the
settingsStoreImpl
branch
from
66399fb to
cb09228
Compare
MatthewTighe
left a comment
MatthewTighe
left a comment
There was a problem hiding this comment.
Nice, had a question and a couple thoughts but nothing blocking. LGTM! thanks for handling this one
There was a problem hiding this comment.
I don't think this is your problem to solve, but just FYI I'm not super happy that we have to have two stores here. This should work, since the "parent" store maintains the state, but it would be good for us as a team to invest in a richer solution at some point.
There was a problem hiding this comment.
Can we file a follow-up to migrate this screen to a Store? I think we've had enough examples of working around the complexity here that it's probably worth just aligning on the common pattern
There was a problem hiding this comment.
Curious to know: do we need to remember this controller or the callbacks object for a specific reason?
|
It does look like the test failures in the try are real though, could you investigate that to make sure? |
3 participants
https://treeherder.mozilla.org/jobs?repo=try&landoInstance=lando-prod-2025&landoCommitID=46701