Refactor: sync useLocalStorage across components and tabs #977

katspaugh · 2022-10-25T11:11:38Z

What it solves

Although the localStorage is global and was shared across all calls of the useLocalStorage hook, it wasn't triggering an update when one of the instances sets a new value.

This PR subscribes to the global storage event and syncs the hooks across components and browser tabs.

github-actions · 2022-10-25T11:15:16Z

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type	Occurrences	Fixable
Errors	0	0
Warnings	0	0
Ignored	0	N/A

Result: ✅ success
Annotations: 0 total

^{Report generated by eslint-plus-action}

cloudflare-pages · 2022-10-25T11:16:09Z

Deploying with Cloudflare Pages

Latest commit:	`7b46ff6`
Status:	✅ Deploy successful!
Preview URL:	https://fcc7b792.web-core.pages.dev
Branch Preview URL:	https://use-storage.web-core.pages.dev

View logs

usame-algan · 2022-10-26T10:41:26Z

src/services/local-storage/useLocalStorage.ts

+      if (event.key === local.getPrefixedKey(key)) {
+        const newValue = local.getItem<T>(key)
+        setCache((oldValue) => {
+          return newValue === undefined || newValue === oldValue ? oldValue : newValue


Do we need to check anything here or can we simply return newValue?

For example if we use the hook twice for the same local storage key and one hook updates its value to undefined, should the other hook not also update its cache to undefined?

The current usage of this hook unfortunately expects that the value is never undefined. I think it would be more correct if you could set it to undefined, but I kept the previous behavior.

In safe creation we set the pendingSafe to undefined when the user cancels the flow. With the change it is being set to {} which causes unintended behaviour. Maybe we could call local.removeItem when undefined is passed?

OK, makes sense, let me allow undefined and remove the initialValue.

src/services/local-storage/useLocalStorage.ts

src/components/create-safe/status/CreationStatus.tsx

usame-algan

💯

* Refactor: external store in useLocalStorage * Comment * Sync separately * Use the storage event for syncing * Rm comment * Restore prev behavior wrt undefined * Simplify the undefined check * Allow undefined and rm initialValue * Don't set undefined * Local storage to return null when not found * Initial value * Rm initial value again * usePendingCreation * Rm redundant code

* feat: AB testing system * fix: remove reset function + test `AbTest` value * Refactor: sync useLocalStorage across components and tabs (#977) * Refactor: external store in useLocalStorage * Comment * Sync separately * Use the storage event for syncing * Rm comment * Restore prev behavior wrt undefined * Simplify the undefined check * Allow undefined and rm initialValue * Don't set undefined * Local storage to return null when not found * Initial value * Rm initial value again * usePendingCreation * Rm redundant code * Refactor: external store in useLocalStorage (#1014) Refactor: external store in useLocalStorage * feat: create `useABTesting` hook for setting event * fix: reorganise structure Co-authored-by: katspaugh <381895+katspaugh@users.noreply.github.com>

* feat: new creation design + step 1 * fix: widget width * refactor: combine components * fix: rename folder + remove CSS module * fix: missing padding * fix: Reuse NetworkSelector component * [Safe Creation] Owner step (#989) * feat: basic step 2 implementation Co-authored-by: iamacook <aaron@safe.global> * feat: creation info widget (#960) * feat: creation info widget * fix: Remove demo page Co-authored-by: Usame Algan <usame@safe.global> * Safe creation stepper (#992) A new CardStepper component which controls the steps for new safe creation which can - go back and forth - jump to a specific step - set an initial step - set initial values - renders a progress bar - step data is stored in an object with a generic type - the update functions (onBack, onSubmit) use Partials of that type such that each step does not need to submit the full data change /demo route to /create-safe Co-authored-by: Usame Algan <usame@safe.global> * Review step redesign (#993) * feat: new create safe review step * calculate total gas fee * fill the form values * refactor review rows * fix: implement Figma designs * fix: background-color on network fee * avoid infinite rerendering * integrate review step in new Stepper * style: visual tweaks * fix: Create safe layout adjustments (#997) * fix: Create safe layout adjustments * fix: overview widget margin top * fix: Move overview widget to top on mobile * feat: AB testing system (#1013) * feat: AB testing system * fix: remove reset function + test `AbTest` value * Refactor: sync useLocalStorage across components and tabs (#977) * Refactor: external store in useLocalStorage * Comment * Sync separately * Use the storage event for syncing * Rm comment * Restore prev behavior wrt undefined * Simplify the undefined check * Allow undefined and rm initialValue * Don't set undefined * Local storage to return null when not found * Initial value * Rm initial value again * usePendingCreation * Rm redundant code * Refactor: external store in useLocalStorage (#1014) Refactor: external store in useLocalStorage * feat: create `useABTesting` hook for setting event * fix: reorganise structure Co-authored-by: katspaugh <381895+katspaugh@users.noreply.github.com> * feat: add AB testing to Safe creation (#1017) * [Create Safe] feat: Connect wallet step (#1006) * fix: Add connect wallet step to safe creation * fix: Remove unused method * fix: Reuse components, adjust modal zIndex * fix: Only run onConnect callback if there was no error when connecting a wallet * fix: Set wallet owner in form on connect * fix: Check for connected wallet * style: Use onboard zindex variable * [Create Safe] add overview and CreateSafeInfo widget (#1002) * add overview and CreateSafeInfo widget - displays safe name and connected wallet during owner setup - displays hints for each step + dynamic hints during owner and threshold setup - state is pulled up into the CreateSafe component and update functions passed into the owner step * step static tips are collapsed by default * navigate tips logic * reverse expanded logic * restyle InfoWidget * remove the add mobile owner row * remove flickering when opening the accordion * fix keys * add hover style to the Accordion expand icon * Accordion summary bold when expanded * style: PR comments * fix empty form data name * simplify map key * link to Safe setup article * change allOwners logic * mv sx to css modules * add current chain in Overview widget Co-authored-by: Diogo Soares <diogo.lsoares@gmail.com> * [Create Safe] feat: Status view redesign (#1018) * feat: Implement create safe status redesign * style: Add animated loading spinner * fix: Simplify conditions * add: animate into css safe logo * fix: use numeric enum for simpler conditions, adjust animated logo sizes * fix: Display dialog after safe creation * fix: Feedback * fix: Better name for query param Co-authored-by: schmanu <manu@safe.global> * [Safe creation] Creation links tooltips (#1061) * add link to tip * add missing tooltips * update copy * use SvgIcon to inherit viewBox * fix: inherit `fill` Co-authored-by: iamacook <aaron@safe.global> * fix: textual inconsistencies + add hint (#1065) * [Safe Creation] Layout adjustments (#1063) * style: Safe Creation layout * fix: Align threshold text * [Create Safe] Handle wallet connection (#1087) * fix: Handle wallet connection when creating safe * fix: Only watch threshold * refactor: Rename useSetCreationStep to useSyncSafeCreationStep * fix: Use formState.isValid * fix: Add tests for useSyncSafeCreationStep * fix: Show error notifications in safe creation (#1135) * fix: Show error notifications in safe creation * fix: Add closeByGroupKey action to notification slice * fix: Safe creation issues (#1180) * fix: Navigate back to the welcome page on cancel * fix: Reuse NameInput for new form * fix: Use currentColor for icons * fix: check for pending safe creation in first step (#1190) * fix: reduce threshold when removing owner (#1181) * fix: reduce threshold when removing owner * fix: use enum for all field names * fix: reduce code * Safe creation text updates (#1198) * fix: Update text and layout for step 0 * fix: Update text for step 2 and 3, show copy and explorer icons for addresses * fix: Update text for status messages * fix: Update hint texts * fix: Update creation modal text, add dynamic values * fix: QR reader crashes app (#1200) * fix: Safe Creation color issues (#1204) * fix: Safe Creation color issues * chore: Add TODO * fix: Remove static colors and use palette directly * fix: Add overview widget to step 1 in safe creation (#1210) * fix: Add missing safe creation events (#1212) * fix: Add missing safe creation events * fix: Add new events for hints * fix: Watch safe creation tx even without a connected wallet (#1215) * fix: Watch safe creation tx even without a connected wallet * fix it in the old safe creation flow too * fix: failing test * fix: Add additional test * fix: Only watch safe creation tx once (#1223) * fix: Only watch safe creation tx once * fix: Detect if tx reverted when ethers throws error * refactor: Remove unused code that was duplicated (#1230) * refactor: Remove unused code that was duplicated * fix: Revert ab test removal * refactor: Remove unused StepCard component * fix: Update links to safe.global * fix: Set link to safe creation for ab testing * fix: Hide sidebar on safe creation * fix: Revert ab test change * fix: Center QR code reload icon * fix: Safe cration text changes (#1239) * fix: Safe cration text changes * fix: Adjust safe indexed message * fix: Adjust safe indexed message Co-authored-by: iamacook <aaron@safe.global> Co-authored-by: Manuel Gellfart <manu@safe.global> Co-authored-by: Diogo Soares <32431609+DiogoSoaress@users.noreply.github.com> Co-authored-by: katspaugh <381895+katspaugh@users.noreply.github.com> Co-authored-by: Diogo Soares <diogo.lsoares@gmail.com>

Refactor: external store in useLocalStorage

e7919a0

katspaugh requested a review from usame-algan October 25, 2022 11:11

Comment

52e9a1f

katspaugh added 3 commits October 25, 2022 13:51

Merge branch 'dev' of github.com:safe-global/web-core into use-storage

4b0b21f

Sync separately

e0f2991

Use the storage event for syncing

173dc02

katspaugh changed the title ~~Refactor: external store in useLocalStorage~~ Refactor: sync useLocalStorage across components and tabs Oct 25, 2022

katspaugh added 2 commits October 26, 2022 10:11

Rm comment

23a68d5

Restore prev behavior wrt undefined

9ae1d24

katspaugh force-pushed the use-storage branch from 1929a11 to 9ae1d24 Compare October 26, 2022 08:18

usame-algan reviewed Oct 26, 2022

View reviewed changes

katspaugh added 6 commits October 26, 2022 13:24

Simplify the undefined check

347a484

Allow undefined and rm initialValue

43f7a3d

Merge branch 'dev' of github.com:safe-global/web-core into use-storage

dbf033f

Don't set undefined

efdce6a

Local storage to return null when not found

08e04b9

Initial value

91ddc78

katspaugh force-pushed the use-storage branch from a118a3e to 91ddc78 Compare October 27, 2022 09:42

Rm initial value again

ce9b97d

iamacook reviewed Oct 28, 2022

View reviewed changes

src/services/local-storage/useLocalStorage.ts Outdated Show resolved Hide resolved

Merge dev

173471c

katspaugh force-pushed the use-storage branch from 2c89cd9 to 790b1bd Compare October 31, 2022 07:42

usePendingCreation

17cd2d8

katspaugh force-pushed the use-storage branch from 790b1bd to 17cd2d8 Compare October 31, 2022 08:11

iamacook approved these changes Oct 31, 2022

View reviewed changes

usame-algan reviewed Oct 31, 2022

View reviewed changes

src/services/local-storage/useLocalStorage.ts Show resolved Hide resolved

src/components/create-safe/status/CreationStatus.tsx Outdated Show resolved Hide resolved

Rm redundant code

7b46ff6

usame-algan approved these changes Oct 31, 2022

View reviewed changes

katspaugh merged commit 5644642 into dev Oct 31, 2022

katspaugh deleted the use-storage branch October 31, 2022 09:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Refactor: sync useLocalStorage across components and tabs #977

Refactor: sync useLocalStorage across components and tabs #977

katspaugh commented Oct 25, 2022 •

edited

github-actions bot commented Oct 25, 2022 •

edited

cloudflare-pages bot commented Oct 25, 2022 •

edited

usame-algan Oct 26, 2022

katspaugh Oct 26, 2022

usame-algan Oct 26, 2022 •

edited

katspaugh Oct 26, 2022

usame-algan left a comment

Refactor: sync useLocalStorage across components and tabs #977

Refactor: sync useLocalStorage across components and tabs #977

Conversation

katspaugh commented Oct 25, 2022 • edited

What it solves

github-actions bot commented Oct 25, 2022 • edited

ESLint Summary View Full Report

cloudflare-pages bot commented Oct 25, 2022 • edited

Deploying with Cloudflare Pages

usame-algan Oct 26, 2022

Choose a reason for hiding this comment

katspaugh Oct 26, 2022

Choose a reason for hiding this comment

usame-algan Oct 26, 2022 • edited

Choose a reason for hiding this comment

katspaugh Oct 26, 2022

Choose a reason for hiding this comment

usame-algan left a comment

Choose a reason for hiding this comment

katspaugh commented Oct 25, 2022 •

edited

github-actions bot commented Oct 25, 2022 •

edited

cloudflare-pages bot commented Oct 25, 2022 •

edited

usame-algan Oct 26, 2022 •

edited