[C- 4926] Add Menu/MenuItem and update Select #9427

dylanjeffers · 2024-08-12T23:13:20Z

Description

Adds internal Menu/MenuItem
Consolidates FilterButton and Select implementation (note there is much more to still improve, in particular building the "option-key-handler" into the Menu)
Implements Select with new system
Updates select props and implementation to handle complex options, up to spec.

TODO: I will add improvements to the KeyFilterInput, since the "value,label,option" interaction is slightly different. It works well enough, but would like to fix before merging.

changeset-bot · 2024-08-12T23:13:23Z

⚠️ No Changeset found

Latest commit: 63eb0c5

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

audius-infra · 2024-08-12T23:24:43Z

Preview this change https://demo.audius.co/c-4926-popup-menu

amendelsohn

Fantastic rewrite!

Needs a bit of cleanup and there's some leftover commented code, but LGTM!

amendelsohn · 2024-08-19T21:01:38Z

packages/harmony/src/components/button/FilterButton/FilterButton.tsx

+            {...option}
            onChange={handleChange}
-            activeValue={activeValue}
+            isActive={option.value === activeValue}


Just confirming: option.value should always be unique, right?
Otherwise this could light up two things at once

amendelsohn · 2024-08-19T21:08:58Z

packages/harmony/src/components/button/FilterButton/types.ts

+  value?: Value | null
+  onChange?: (value: Value) => void


missing docstrings

menuProps as well

amendelsohn · 2024-08-19T21:10:02Z

packages/harmony/src/components/input/TextInput/TextInput.tsx

    const placeholderText =
      required && hideLabel
        ? `${placeholder} *`
-        : TextInputSize.SMALL


amendelsohn · 2024-08-19T21:10:41Z

packages/harmony/src/components/input/TextInput/types.ts

   */
  hideLabel?: boolean
+
+  hidePlaceholder?: boolean


amendelsohn · 2024-08-19T21:11:48Z

packages/harmony/src/components/internal/Menu.tsx

+export type MenuProps = Omit<PopupProps, 'children'> & {
+  maxHeight?: CSSObject['maxHeight']
+  children: ReactNode
+  PaperProps?: Partial<PaperProps> & { css?: CSSObject }


is there no WithCSS<T> or an existing emotion type for { css?: CSSObject } we can &?

great idea, added this utility, lmk what you think.

amendelsohn · 2024-08-19T21:27:21Z

packages/harmony/src/components/select/Select/Select.tsx

+          label?.toLowerCase().includes(inputValue.toLowerCase()) ||
+          value.toLowerCase().includes(inputValue.toLowerCase())


any case for fuzzy find here?

product Q, but maybe worth considering since we'll be using this to search lists of titles like all your purchasable content

also concerned about matching against the values here. If we're searching tracks, and I input '5', do I really want all tracks with ids containing '5'?

great question! i think you are right, the reason for this was that we use the value if there is no label, but to your point, im going to do a label ?? value and then just check for includes on that.

amendelsohn · 2024-08-19T21:31:47Z

packages/harmony/src/components/select/Select/Select.tsx

+            top: 26,
+            left: 4,


do these relate to spacing values?

they are fairly one-off, but i can use spacing values. this was to get the overlay working correctly

amendelsohn · 2024-08-19T21:31:55Z

packages/harmony/src/components/select/Select/Select.tsx

+        hidePlaceholder={!!selectedOption}
+        {...other}
+      />
+      {/* Overlay selected option */}


amendelsohn · 2024-08-19T21:34:33Z

packages/harmony/src/components/select/Select/types.ts

  /**
-   * Popup portal location passed to the inner popup
+   * A function to handle when the value is changed
   */
-  popupPortalLocation?: HTMLElement
-
-  /**
-   * zIndex applied to the inner Popup component
-   */
-  popupZIndex?: number
+  onChange: (value: Value) => void
+  options: ReactNode


idk if either is necessary, but odd to have one docstring and not the other, and below in SelectProps, it's the opposite

amendelsohn · 2024-08-19T21:35:57Z

packages/web/src/components/edit/fields/KeySelectField.tsx

 }

+const getValueFromKey = (key: string) =>
+  // If the key is an enharmonic equivalent (e.g. C# and Db), use the flat as the value


🔥 💯 👍

gitguardian · 2024-08-20T14:07:47Z

⚠️ GitGuardian has uncovered 11 secrets following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secrets in your pull request

GitGuardian id	GitGuardian status	Secret	Commit	Filename
9412812	Triggered	Generic Password	`63eb0c5`	core/infra/docker-compose.yml	View secret
11648676	Triggered	Generic High Entropy Secret	`63eb0c5`	core/infra/dev_config/discovery-one.docker.env	View secret
11648676	Triggered	Generic High Entropy Secret	`63eb0c5`	core/infra/dev_config/sandbox-one.env	View secret
11648676	Triggered	Generic High Entropy Secret	`63eb0c5`	core/infra/dev_config/discovery-one.env	View secret
11648678	Triggered	Generic High Entropy Secret	`63eb0c5`	core/infra/dev_config/content-three.docker.env	View secret
11648678	Triggered	Generic High Entropy Secret	`63eb0c5`	core/infra/dev_config/content-three.env	View secret
11648679	Triggered	Generic High Entropy Secret	`63eb0c5`	core/infra/dev_config/content-two.env	View secret
11648679	Triggered	Generic High Entropy Secret	`63eb0c5`	core/infra/dev_config/content-two.docker.env	View secret
11648680	Triggered	Generic High Entropy Secret	`63eb0c5`	core/infra/dev_config/content-one.docker.env	View secret
11648680	Triggered	Generic High Entropy Secret	`63eb0c5`	core/infra/dev_config/content-one.env	View secret
13407071	Triggered	Generic High Entropy Secret	`63eb0c5`	comms/Makefile	View secret

🛠 Guidelines to remediate hardcoded secrets

Understand the implications of revoking this secret by investigating where it is used in your code.
Replace and store your secrets safely. Learn here the best practices.
Revoke and rotate these secrets.
If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider

following these best practices for managing and storing secrets including API keys and other credentials
install secret detection on pre-commit to catch secret before it leaves your machine and ease remediation.

^{🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.}

audius-infra · 2024-08-20T14:22:59Z

Preview this change https://demo.audius.co/c-4926-popup-menu

[1ba94b8] Reland "[PAY-3357] Migrate Notifications to SDK" (#9539) Marcus Pasell [c33b4e3] Revert "[PAY-3357] Migrate Notifications to SDK" (#9538) Marcus Pasell [b8c360e] [PAY-3357] Migrate Notifications to SDK (#9516) Marcus Pasell [eaef9fd] [QA-1498] Fix long press on DMs (#9533) JD Francis [4bbfeff] [PAY-3373] Purchasers endpoint (#9525) Reed [72ff6a3] [C-4909] Improve track metadata list, fix album backlinks (#9531) Dylan Jeffers [e5248b7] [QA-1515] Remove dismiss on leave from popup (#9527) JD Francis [46305bb] [PAY-2918] Migrate track endpoints to SDK - Round 3 (#9522) Randy Schott [b957a52] Fix challenge api tx confirming (#9526) Isaac Solo [6999b53] [QA-1519] Fix buyer metadata caching (#9512) Isaac Solo [7f17c95] Hook up remixers count to web blast UI (#9523) Reed [75ab270] [PAY-3359] Chat blast thread UI on sender side (#9515) Reed [93e3ac9] Mobile comment fixes (#9507) Sebastian Klingler [c81ed5d] [QA-1499] Fix long usernames overflowing manager mode button (#9519) JD Francis [7763deb] Add react-router dependency (#9520) Isaac Solo [6de56bd] Move trpc to common (#9518) Dylan Jeffers [76d4095] Active styles for web chat blasts (#9517) Reed [3464a2a] Add row and column props to flex (#9505) Dylan Jeffers [e6b1da1] [C-4902] Add harmony checkbox (#9510) Dylan Jeffers [0fcf9e4] [PAY-3361] ChatBlastSelectAudienceScreen (#9502) Andrew Mendelsohn [2566eb8] [PAY-3228] Implement composer unfurl in mobile (#9504) Raymond Jacobson [b5bf00a] [QA-1210] Fix popup scrolling issues (#9508) JD Francis [5349315] Update logic for allowing chat blast sending (#9509) Reed [51391d9] [PAY-3362] Move link resolver to common hook (#9503) Raymond Jacobson [45dc83d] Add feature flag refresh (#9506) Dylan Jeffers [3af4e69] [PAY-2918] Migrate track endpoints to SDK, Round 2 (#9494) Randy Schott [78897c4] [PAY-3355] Address link preview / unfurl QA items (#9492) Raymond Jacobson [c2a9387] [PAY-3366] Fix CTA hidden on small screens (#9499) Andrew Mendelsohn [e6dbb7a] Fix CTA empty states reversed web/mobile (#9498) Andrew Mendelsohn [f3d7825] Remove listenUserId from chat api (#9479) Reed [d55dbb1] [PAY-3360] Chat Blast CTA (mobile) (#9491) Andrew Mendelsohn [adb99e3] Fix challenge claim notification amount (#9485) Isaac Solo [d4518ce] [C- 4926] Add Menu/MenuItem and update Select (#9427) Dylan Jeffers [b914ade] [PAY-2918] Migrate tracks endpoints to SDK - round 1 (#9474) Randy Schott [893ed42] Revamp local solana-test-validator (#9482) Marcus Pasell [7ff4ab0] Separate saga top level error handling (#9458) Raymond Jacobson [06a19e6] Remote config for chat blast tier requirement (#9490) Reed [2b804b2] [PAY-3345] Prevent artist reacting to their own blasts (#9488) Reed [1cc1915] [C-4976] Refactor comment hooks (#9489) Sebastian Klingler [b3a4c77] [QA-1511] Fix space & arrow hotkeys not working (#9486) JD Francis [1342d1f] [C-4823] Mobile comment drawer (#9478) Sebastian Klingler [075d757] [QA-1514] Fix search filter typing (#9484) Sebastian Klingler [bfaa536] Enabling blocking users from commenting (#9469) Isaac Solo [c2d5e85] Bump mobile apps to 111 (#9481) Dylan Jeffers [5944ced] [PAY-3358] Fire chat message RPCs on blast write (#9476) Reed [83c97ed] [C-4972] Add comment sort bar (#9477) JD Francis

dylanjeffers added 3 commits August 12, 2024 13:26

[C-4926] Add popup-menu and consolidate

3b0d7d1

Add select

8799018

Update usages

fe649b8

pull-request-size bot added the size/XXL label Aug 12, 2024

dylanjeffers requested a review from amendelsohn August 12, 2024 23:13

amendelsohn approved these changes Aug 19, 2024

View reviewed changes

dylanjeffers added 2 commits August 20, 2024 15:48

Address changes

ab65b16

Merge branch 'main' into c-4926-popup-menu

63eb0c5

dylanjeffers merged commit d4518ce into main Aug 20, 2024

dylanjeffers deleted the c-4926-popup-menu branch August 20, 2024 14:36

This was referenced Sep 16, 2024

[C-5020] Search filter fixes for FilterButton #9763

Merged

Fix harmony tests #9775

Merged

		label?.toLowerCase().includes(inputValue.toLowerCase()) \|\|
		value.toLowerCase().includes(inputValue.toLowerCase())

[C- 4926] Add Menu/MenuItem and update Select #9427

[C- 4926] Add Menu/MenuItem and update Select #9427

Uh oh!

Conversation

dylanjeffers commented Aug 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

changeset-bot bot commented Aug 12, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ No Changeset found

Uh oh!

audius-infra commented Aug 12, 2024

Uh oh!

amendelsohn left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dylanjeffers Aug 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gitguardian bot commented Aug 20, 2024

⚠️ GitGuardian has uncovered 11 secrets following the scan of your pull request.

Uh oh!

audius-infra commented Aug 20, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dylanjeffers commented Aug 12, 2024 •

edited

Loading

changeset-bot bot commented Aug 12, 2024 •

edited

Loading

dylanjeffers Aug 20, 2024 •

edited

Loading