[C-3362] Port Popup component to Harmony #6685

raymondjacobson · 2023-11-14T06:23:05Z

Description

Port Stems Popup componet to harmony.
Actual diff:
https://www.diffchecker.com/WAGLkhwA/

Fixed a couple issues:

Popup doesn't work when portaled element is what scrolls (e.g. the document.body in the inner iframe in storybook)
Delete deprecated position

How Has This Been Tested?

Please describe the tests that you ran to verify your changes. Provide repro instructions & any configuration.

npm run storybook

audius-infra · 2023-11-14T17:00:06Z

Preview this change https://demo.audius.co/rj-c-3362

nicoback2 · 2023-11-15T18:46:55Z

packages/harmony/src/components/layout/Popup/Popup.module.css

+  background-color: var(--harmony-white);
+  border-radius: var(--harmony-unit-2);
+  border: 1px solid var(--harmony-neutral-light-7);
+  box-shadow: 0 16px 20px 0 var(--harmony-tile-shadow-1-alt);


i don't think this exists, ideally we'd use one of the harmony shadow values here https://www.figma.com/file/ZmEdR1D99gglpBgiZDqWrr/%F0%9F%A7%B1-Foundations?node-id=1929%3A4771&mode=dev

nicoback2 · 2023-11-15T18:51:07Z

packages/harmony/src/components/layout/Popup/Popup.module.css

+
+.popup {
+  background-color: var(--harmony-white);
+  border-radius: var(--harmony-unit-2);


Suggested change

border-radius: var(--harmony-unit-2);

border-radius: var(--harmony-border-radius-m);

https://www.figma.com/file/ZmEdR1D99gglpBgiZDqWrr/%F0%9F%A7%B1-Foundations?node-id=1935%3A54403&mode=dev

nicoback2 · 2023-11-15T18:51:24Z

packages/harmony/src/components/layout/Popup/Popup.module.css

@@ -0,0 +1,47 @@
+.root {
+  font-family: var(--harmony-font-family);


do we need this?

nicoback2 · 2023-11-15T18:53:49Z

packages/harmony/src/components/layout/Popup/Popup.module.css

+.popup {
+  background-color: var(--harmony-white);
+  border-radius: var(--harmony-unit-2);
+  border: 1px solid var(--harmony-neutral-light-7);


Suggested change

border: 1px solid var(--harmony-neutral-light-7);

border: 1px solid var(--harmony-border-strong);

we use a different system for neutrals now, it's harmony-n-xxx instead of harmony-neutral-light-x so for e.g. neutral-light-7 is now harmony-neutral-150 which is the same as harmony-border-strong (semantic)

nicoback2 · 2023-11-15T18:54:48Z

packages/harmony/src/components/layout/Popup/Popup.module.css

+
+.header {
+  border: none;
+  margin-bottom: var(--harmony-unit-1);


i think we generally prefer semantic spacing when possible so like --harmony-spacing-xs for this one

nicoback2 · 2023-11-15T19:15:01Z

packages/harmony/src/components/layout/Popup/Popup.tsx

+                  <div
+                    className={cn(styles.header, {
+                      [styles.noAfter]: hideCloseButton
+                    })}


Should use Flex instead and then can remove the styles.header styles

<Flex mb='xs' alignItems='center' justifyContent='center'>

nicoback2 · 2023-11-15T19:20:26Z

packages/harmony/src/components/layout/Popup/Popup.tsx

+                        iconLeft={IconClose}
+                      />
+                    )}
+                    <div className={styles.title}>{title}</div>


should use Text component for the text here and then can delete the font and color styles from styles.title class

nicoback2 · 2023-11-15T19:23:26Z

packages/harmony/src/components/layout/Popup/Popup.stories.tsx

+
+export default meta
+
+type Story = StoryObj<typeof Popup>


would be good to have some more stories here with different props applied, but can leave it for later

nicoback2 · 2023-11-15T19:24:53Z

packages/harmony/src/components/layout/Popup/Popup.tsx

+ * Number of pixels between the edge of the container and the popup
+ * before the popup needs to reposition itself to be in view.
+ */
+const CONTAINER_INSET_PADDING = 16


const CONTAINER_INSET_PADDING = spacing.l

(import {spacing} from 'packages/harmony/src/foundations/spacing/spacing.ts')

nicoback2 · 2023-11-15T19:29:26Z

packages/harmony/src/components/layout/Popup/Popup.tsx

+                    })}
+                  >
+                    {hideCloseButton ? null : (
+                      <PlainButton


Does this end up looking right? I don't think PlainButton is meant to be used for icons without text, but I guess it would work. Ideally we'd use IconButton but we don't have that implemented yet (I ran into this same issue and ended up just building a one off https://github.com/AudiusProject/audius-protocol/blob/main/packages/harmony/src/components/input/PasswordInput/PasswordInput.tsx#L30-L49)

raymondjacobson · 2023-11-15T20:45:06Z

Going to follow up on these comments all in subsequent PR as it's blocking the FilterButton & some other downstream work. Will circle back with another change! ty @nicoback2

[a917c79] Fix MetaMask not working C-3386 (#6738) nicoback2 [58b4140] Implement sign up + sign in with Metamask C-3356 (#6732) nicoback2 [6956101] [PAY-2174] Support mobile web properly in add funds flows (#6740) Raymond Jacobson [cd09949] Support FixedDecimal on mobile (#6739) Marcus Pasell [cc6d1c7] [PAY-2173] Make payments page work nicely in mobile web (#6726) Raymond Jacobson [a2e38c2] [PAY-2152] Bring add funds up to spec (web) (#6725) Raymond Jacobson [48f90ca] [C-3383] Update Divider (#6728) Dylan Jeffers [170634d] [C-3237] Sign Up Mobile Nav Header (#6730) JD Francis [bcbe2d5] Tweak Harmony Icon Docs (#6733) JD Francis [c152f0e] [PAY-2158] Update purchase-content saga to account for purchase method (#6737) Randy Schott [bb4beae] Fix metamask help url (#6736) Isaac Solo [4706195] [PAY-2156] Purchase flow web ui updates (#6718) Reed [682e572] [PAY-2168] Add better typechecking for FixedDecimal (#6724) Marcus Pasell [5e8f8b7] [C-3337] Fix table section end styles (#6731) Kyle Shanks [e69bdf5] [C-3270] Mobile UI for create email page (#6701) JD Francis [6a507f5] Bump version to 0.5.19 audius-infra [47910e2] [PAY-2159] Make purchase summary collapsible on web (#6719) Randy Schott [f7162e7] [C-3364] FollowButton (#6708) Andrew Mendelsohn [d6cad0a] Update modal title and icon colors (#6721) Dylan Jeffers [9b492af] Add accent color to text and icons (#6720) Dylan Jeffers [41dc703] Upgrade wagmi to v2 (in /d) (#6712) Theo Ilie [5061bee] Rename /payandearn to /payments (#6717) Raymond Jacobson [664a1aa] Fixed package-lock.json to not have broken bn types in @audius/fixed-decimal (#6716) Marcus Pasell [c02dfb3] [C-3363] FollowArtistsPage initial layout (#6692) Andrew Mendelsohn [5e64792] [C-3378] Improve web-app hierarchy (#6710) Dylan Jeffers [bc48098] Patch verified user on creation (#6705) Isaac Solo [f0d7a57] Fix icon color/layout in summary table (#6715) Raymond Jacobson [96f418b] [PAY-2164] Update web nav usdc and audio (#6696) Saliou Diallo [ed26ea6] [C-3353] Add WelcomeModal for the new sign up flow for both web and mobile web (#6707) Kyle Shanks [026aead] [PAY-2154] Implement add funds modal (#6714) Raymond Jacobson [68aa7b5] Bump version to 0.5.18 audius-infra [c35bac5] [PROTO-1426] Display chain+GH versions with react-query (#6704) Theo Ilie [19b532c] Fix protodash mobile nav services & governance (#6709) Theo Ilie [e8ebbc6] Update docs for upload track mood (#6706) Isaac Solo [27c8aa4] [PROTO-1398] Add audius-d sandbox-compatible startup to mediorum (#6693) Danny [d584a9e] [C-3375] Update TextLink styles and examples (#6703) Dylan Jeffers [5d237e8] [PAY-2161] Implement FilterButton in Harmony (#6697) Raymond Jacobson [d6e8c15] Healthz explorer (#6664) Steve Perkins [d16b941] Fix 10th track has 15s wait bug (#6702) Reed [ca5bf83] [C-3362] Port Popup component to Harmony (#6685) Raymond Jacobson [902e805] [PROTO-1430] Add external wallets to /d (fka /up) (#6695) Theo Ilie [6947471] [C-3370] Fix harmony types (#6694) Dylan Jeffers [55f2957] Bump version to 0.5.17 audius-infra [7ec407d] [PAY-2165][PAY-2163] Add payments and earnings to mobile (#6688) Saliou Diallo [33e22f1] [C-3223] Add select-genre sign-up page (#6691) Dylan Jeffers [5302154] Sales, Purchases, Withdrawals tables on Pay & Earn Page (#6684) Reed [a5c1cf8] [C-3369] Add checkbox/radio types to SelectablePill (#6689) Dylan Jeffers [9a83de5] Migrate release_date from string to date (#6655) Isaac Solo [c7445f9] [PAY-2167] @audius/fixed-decimal: A data structure for fixed precision decimals (#6662) Marcus Pasell [bdee772] Fix harmony docs sidebar-item (#6690) Dylan Jeffers [5dffb42] Fix Flex prop leaking, Upgrade Box (#6686) Dylan Jeffers [ae9e668] Hotfix double gzip on mobile for Content Nodes (#6687) Theo Ilie [48154d5] Add pay-gated upload test and re-enable stems test (#6681) Raymond Jacobson [bb0f2a4] Bump version to 0.5.16 audius-infra [7937393] Add text-shadow, fix avatar className, add 2xs spacing (#6682) Dylan Jeffers [0f0429a] Fix broken useAccountHasClaimableRewards (#6683) Reed [e5da54d] [PROTO-1418] Add dynamically imported Audius libs to /up app (#6680) Theo Ilie [5bb8a96] [PAY-1955] Fix purchases, sales, withdrawals text overflow (#6669) Saliou Diallo [a75a1e5] [C-3294] Add Harmony PlainButton docs (#6678) Kyle Shanks [445cec1] Fix typography header xl font-size and line-height (#6677) Dylan Jeffers [e5d53dc] Ignore animations in Chromatic (#6676) Dylan Jeffers [c91486c] [C-3278] Add TextLink component and docs (#6661) Andrew Mendelsohn [9b4a017] Fix emotion support in web (#6675) Dylan Jeffers [c474b2a] [Sign up Redesign] Sign up white screens footer (web); add shadow mid inver to Harmony C-3358 C-3359ted (#6671) nicoback2 [0b0aedd] [INF-546] Ensure that vite base is set properly in prod builds (#6674) Sebastian Klingler [29f1466] [C-3244] Fix heading text clipping (#6670) Dylan Jeffers [57b8ddc] Only announce prod client deploys (#6673) Raymond Jacobson [ef80e97] Add back react-native symlink step in CI (#6672) Sebastian Klingler [e52b8c2] Add no-constant-binary-expression eslint rule (#6666) Sebastian Klingler [02b28d7] Fix trpc endpoints w/ vite (#6668) Sebastian Klingler [c358e17] Disable ToS banner (#6665) Raymond Jacobson [183434d] [PAY-2151] Create Pay & Earn Page (#6654) Reed [e313506] Fix vite webworker basename issue (#6667) Raymond Jacobson [3ded93c] [C-3274, C-3348] Add README to Harmony (#6663) Kyle Shanks [5ff8e61] Bump version to 0.5.15 audius-infra

pull-request-size bot added the size/XL label Nov 14, 2023

raymondjacobson added 3 commits November 14, 2023 08:48

[C-3362] Port Popup component to Harmony

cca335a

Remove paper in demo

32f577b

Clean up and fix lint

d0c30a3

raymondjacobson force-pushed the rj-c-3362 branch from 9e55c5d to d0c30a3 Compare November 14, 2023 16:48

raymondjacobson requested review from a team and nicoback2 and removed request for a team November 14, 2023 17:39

raymondjacobson mentioned this pull request Nov 15, 2023

[PAY-2161] Implement FilterButton in Harmony #6697

Merged

nicoback2 approved these changes Nov 15, 2023

View reviewed changes

raymondjacobson merged commit ca5bf83 into main Nov 15, 2023
14 of 16 checks passed

raymondjacobson deleted the rj-c-3362 branch November 15, 2023 20:45

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[C-3362] Port Popup component to Harmony #6685

[C-3362] Port Popup component to Harmony #6685

raymondjacobson commented Nov 14, 2023 •

edited

Loading

audius-infra commented Nov 14, 2023

nicoback2 Nov 15, 2023

nicoback2 Nov 15, 2023

nicoback2 Nov 15, 2023

nicoback2 Nov 15, 2023

nicoback2 Nov 15, 2023

nicoback2 Nov 15, 2023

nicoback2 Nov 15, 2023

nicoback2 Nov 15, 2023

nicoback2 Nov 15, 2023

nicoback2 Nov 15, 2023

raymondjacobson commented Nov 15, 2023

	border-radius: var(--harmony-unit-2);
	border-radius: var(--harmony-border-radius-m);

		@@ -0,0 +1,47 @@
		.root {
		font-family: var(--harmony-font-family);

	border: 1px solid var(--harmony-neutral-light-7);
	border: 1px solid var(--harmony-border-strong);

[C-3362] Port Popup component to Harmony #6685

[C-3362] Port Popup component to Harmony #6685

Conversation

raymondjacobson commented Nov 14, 2023 • edited Loading

Description

How Has This Been Tested?

audius-infra commented Nov 14, 2023

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

raymondjacobson commented Nov 15, 2023

raymondjacobson commented Nov 14, 2023 •

edited

Loading