[Create Safe] feat: Status view redesign #1018

usame-algan · 2022-11-01T11:05:41Z

What it solves

Resolves #994

How this PR fixes it

New design for the status view that includes a stepper
Animated loading spinner
Manual navigation to the safe after creation
A new Dialog with first step hints after creation

How to test it

Open /new-safe/create
Submit the form
Observe the new design

ToDos

Add loading spinner with animations
Add modal on dashboard after safe creation
Review Dialog content and status messages cc @johannesmoormann

Screenshots

cloudflare-workers-and-pages · 2022-11-01T11:08:14Z

Deploying with Cloudflare Pages

Latest commit:	`5d1e68c`
Status:	✅ Deploy successful!
Preview URL:	https://e52cbe5e.web-core.pages.dev
Branch Preview URL:	https://create-safe-status.web-core.pages.dev

View logs

github-actions · 2022-11-01T11:08:39Z

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}

iamacook · 2022-11-03T11:48:21Z

src/components/dashboard/CreationDialog/index.tsx

+          <HintItem
+            Icon={HomeIcon}
+            title="Home"
+            description="Get a status overview of your Safe and more at your Safe home."


Suggested change

description="Get a status overview of your Safe and more at your Safe home."

description="Get a status overview of your Safe and more on your Safe homepage."

iamacook · 2022-11-03T11:48:36Z

src/components/dashboard/CreationDialog/index.tsx

+          <HintItem
+            Icon={TransactionIcon}
+            title="Transactions"
+            description="Review, approve, execute, keep track of assets movement."


Suggested change

description="Review, approve, execute, keep track of assets movement."

description="Review, approve, execute and keep track of asset movement."

iamacook · 2022-11-03T11:48:55Z

src/components/dashboard/CreationDialog/index.tsx

+          <HintItem
+            Icon={AppsIcon}
+            title="Apps"
+            description="Over 70 dApps available for you on Mainnet. Check out Safe native apps for secure integrations. "


Suggested change

description="Over 70 dApps available for you on Mainnet. Check out Safe native apps for secure integrations. "

description="Over 70 dApps available for you on Mainnet. Check out native Safe Apps for secure integrations. "

iamacook · 2022-11-03T11:50:06Z

src/components/new-safe/CreateSafe/index.tsx

@@ -71,6 +82,7 @@ const CreateSafe = () => {
    mobileOwners: [] as NamedAddress[],
    owners: [defaultOwner],
    threshold: 1,
+    saltNonce: 0,


Is it ok to default this to 0 or should it be Date.now() instead?

Lets use Date.now() for consistency.

iamacook · 2022-11-03T11:51:52Z

src/components/new-safe/CreateSafe/useCreateSafe.ts

+      setStep(0)
+    }
+
+    // Jump to the status screen if there is already a tx submitted


Maybe this should be use useLayoutEffect instead to prevent any flickers. What do you think?

I tried useLayoutEffect but still get the initial flicker. Another thing I looked into was setting the initialStep depending on pendingSafe but it also doesn't work because our useLocalStorage is not render blocking.

Alright, let's not worry about it.

iamacook · 2022-11-03T11:56:33Z

src/components/new-safe/steps/Step4/StatusMessage.tsx

+    case SafeCreationStatus.WALLET_REJECTED:
+      return {
+        description: 'Transaction was rejected.',
+        instruction: 'You can cancel or retry the Safe creation process.',


Suggested change

instruction: 'You can cancel or retry the Safe creation process.',

instruction: 'Please cancel or retry the Safe creation process.',

I'd change the other occurances here as well. It's repeated every time so you could even move it to a variable.

iamacook · 2022-11-03T11:58:48Z

src/components/new-safe/steps/Step4/logic/index.ts

+
+  // Go to the dashboard if no specific redirect is provided
+  if (!redirectUrl) {
+    return { pathname: AppRoutes.home, query: { safe: address, creation: 1 } }


What is the creation search parameter used for?

To display the modal on the dashboard.

Alright, I would asjust the name/value a bit and add a comment as it is not clear of face value, e.g. { showCreationModal: true }. The router will parse it to a boolean for us.

iamacook · 2022-11-03T11:59:32Z

src/components/new-safe/steps/Step4/logic/index.ts

+  // Otherwise, redirect to the provided URL (e.g. from a Safe App)
+
+  // Track the redirect to Safe App
+  if (redirectUrl.includes('apps')) {


I'd try to be be a bit more specific here as we could have other routes that contain apps.

I originally copied this function from the old create-safe flow but to avoid code duplication I removed it now and added TODOs so we can refactor it at a later time.

iamacook · 2022-11-03T12:00:44Z

src/components/new-safe/steps/Step4/logic/index.ts

+  const hasQueryParams = redirectUrl.includes('?')
+  const appendChar = hasQueryParams ? '&' : '?'
+  return redirectUrl + `${appendChar}safe=${address}`


Can we use URL and/or URLSearchParams here instead of the manual comparison?

I originally copied this function from the old create-safe flow but to avoid code duplication I removed it now and added TODOs so we can refactor it at a later time.

iamacook · 2022-11-03T12:02:22Z

src/components/new-safe/steps/Step4/useSafeCreation.ts

+export enum SafeCreationStatus {
+  AWAITING = 1,
+  PROCESSING = 2,
+  WALLET_REJECTED = 3,
+  ERROR = 4,
+  REVERTED = 5,
+  TIMEOUT = 6,
+  SUCCESS = 7,
+  INDEXED = 8,
+  INDEX_FAILED = 9,
+}


Can you add a comment explaining that these reference steps? On intial sight I was confused as to why we assign a value directly and not use the default from 0.

Good point, we don't need to explicitely set the numbers as its fine to start at 0

DiogoSoaress

It looks good! Left couple of minor comments.
Aaand hats off in the animation! 🔥🫡

DiogoSoaress · 2022-11-04T00:09:03Z

src/components/new-safe/CreateSafe/useCreateSafe.ts

  const wallet = useWallet()
  const isWrongChain = useIsWrongChain()

  const isConnected = wallet && !isWrongChain

+  useEffect(() => {


What do you think of separating this useEffect and the isConnected logic in two hooks? I suggest useIsConnected and useSetCreationStep

Good idea, we could even reuse useIsConnected in other places. I also like @iamacook suggestion to have a context as to not having to include this hook in every component.

I opted for using 2 hooks.

DiogoSoaress · 2022-11-04T00:19:13Z

src/components/new-safe/steps/Step4/useSafeCreation.ts

+  const [isCreating, setIsCreating] = useState<boolean>(false)
+  const [isWatching, setIsWatching] = useState<boolean>(false)


Suggested change

const [isCreating, setIsCreating] = useState<boolean>(false)

const [isWatching, setIsWatching] = useState<boolean>(false)

const [isCreating, setIsCreating] = useState(false)

const [isWatching, setIsWatching] = useState(false)

The type is already inferred because of the initialization

# Conflicts: # src/components/new-safe/CreateSafe/index.tsx # src/components/new-safe/steps/Step1/index.tsx # src/components/new-safe/steps/Step2/index.tsx

* 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>

feat: Implement create safe status redesign

1939751

usame-algan changed the base branch from dev to creation November 1, 2022 11:05

usame-algan and others added 6 commits November 1, 2022 14:11

style: Add animated loading spinner

69a1b6b

fix: Simplify conditions

4c91542

add: animate into css safe logo

1d4efb7

Merge remote-tracking branch 'origin/creation' into create-safe-status

e4d4202

fix: use numeric enum for simpler conditions, adjust animated logo sizes

d367788

fix: Display dialog after safe creation

613c732

usame-algan marked this pull request as ready for review November 2, 2022 16:27

usame-algan requested review from DiogoSoaress and iamacook November 3, 2022 11:17

iamacook reviewed Nov 3, 2022

View reviewed changes

DiogoSoaress approved these changes Nov 4, 2022

View reviewed changes

usame-algan added 2 commits November 4, 2022 11:30

fix: Feedback

6700a5f

Merge remote-tracking branch 'origin/creation' into create-safe-status

4b65f5c

# Conflicts: # src/components/new-safe/CreateSafe/index.tsx # src/components/new-safe/steps/Step1/index.tsx # src/components/new-safe/steps/Step2/index.tsx

usame-algan requested a review from iamacook November 4, 2022 10:35

fix: Better name for query param

5d1e68c

iamacook approved these changes Nov 4, 2022

View reviewed changes

usame-algan merged commit 872f5f9 into creation Nov 4, 2022

usame-algan deleted the create-safe-status branch November 4, 2022 11:27

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Create Safe] feat: Status view redesign #1018

[Create Safe] feat: Status view redesign #1018

usame-algan commented Nov 1, 2022 •

edited

Loading

cloudflare-workers-and-pages bot commented Nov 1, 2022 •

edited

Loading

github-actions bot commented Nov 1, 2022 •

edited

Loading

iamacook Nov 3, 2022

iamacook Nov 3, 2022

iamacook Nov 3, 2022

iamacook Nov 3, 2022

usame-algan Nov 3, 2022

iamacook Nov 3, 2022

usame-algan Nov 4, 2022

iamacook Nov 4, 2022

iamacook Nov 3, 2022

iamacook Nov 3, 2022

iamacook Nov 3, 2022

usame-algan Nov 4, 2022

iamacook Nov 4, 2022

iamacook Nov 3, 2022

usame-algan Nov 4, 2022

iamacook Nov 3, 2022

usame-algan Nov 4, 2022

iamacook Nov 3, 2022

usame-algan Nov 4, 2022

DiogoSoaress left a comment

DiogoSoaress Nov 4, 2022

usame-algan Nov 4, 2022

usame-algan Nov 4, 2022

DiogoSoaress Nov 4, 2022

DiogoSoaress Nov 4, 2022

	description="Get a status overview of your Safe and more at your Safe home."
	description="Get a status overview of your Safe and more on your Safe homepage."

	description="Review, approve, execute, keep track of assets movement."
	description="Review, approve, execute and keep track of asset movement."

	description="Over 70 dApps available for you on Mainnet. Check out Safe native apps for secure integrations. "
	description="Over 70 dApps available for you on Mainnet. Check out native Safe Apps for secure integrations. "

	instruction: 'You can cancel or retry the Safe creation process.',
	instruction: 'Please cancel or retry the Safe creation process.',

		const [isCreating, setIsCreating] = useState<boolean>(false)
		const [isWatching, setIsWatching] = useState<boolean>(false)

[Create Safe] feat: Status view redesign #1018

[Create Safe] feat: Status view redesign #1018

Conversation

usame-algan commented Nov 1, 2022 • edited Loading

What it solves

How this PR fixes it

How to test it

ToDos

Screenshots

cloudflare-workers-and-pages bot commented Nov 1, 2022 • edited Loading

Deploying with Cloudflare Pages

github-actions bot commented Nov 1, 2022 • edited Loading

ESLint Summary View Full Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DiogoSoaress left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

usame-algan commented Nov 1, 2022 •

edited

Loading

cloudflare-workers-and-pages bot commented Nov 1, 2022 •

edited

Loading

github-actions bot commented Nov 1, 2022 •

edited

Loading