Page MenuHomePhabricator

[web] Dropdown component improvements
ClosedPublic

Authored by ginsu on Mon, Jun 24, 9:29 PM.
Tags
None
Referenced Files
F2152495: D12561.diff
Sun, Jun 30, 3:19 PM
F2150611: D12561.id41672.diff
Sun, Jun 30, 10:44 AM
F2143897: D12561.diff
Sat, Jun 29, 6:36 PM
F2143889: D12561.id41817.diff
Sat, Jun 29, 6:34 PM
F2143884: D12561.id41816.diff
Sat, Jun 29, 6:34 PM
F2143391: D12561.id41671.diff
Sat, Jun 29, 4:36 PM
Unknown Object (File)
Fri, Jun 28, 11:17 AM
Unknown Object (File)
Tue, Jun 25, 6:27 AM
Subscribers

Details

Summary

In preperation for using the Dropdown component with the tag farcaster channel modals, I needed to extend the Dropdown component. Here is everything new with the Dropdown component

  1. Introduced defaultLabel prop. This prop is the text/label displayed to the user in the dropdown if no option is currently selected
  2. activeSelection prop is now optional. There will be some cases where initially a dropdown won't have a selection, so we should make this prop optional
  3. Introduced selectedIndex state. Rather than iterating through the list of options every time the activeSelection changes, we can keep track of the index of the activeSelection in the list of options and use that to immediately get the active displayed option
  4. dropdownList is now positioned with absolute rather than relative. The dropdown list was causing the modal to change size whenever the dropdown was opened or closed. With the dropdownList using absolute position, the dropdown list won't affect the size of the modal. One thing to point out is now there could be overflow with the dropdown list; however this seems to be an acceptable pattern in other places that use a dropdown component

Screenshot 2024-06-25 at 1.14.04 AM.png (936×1 px, 538 KB)

Test Plan

Confirmed there were no regressions with the dropdown in the change role modal (only place we use the dropdown component today)

Also confirmed that the dropdown list overflow works as expected

Before:

After:

Screenshot 2024-06-25 at 1.12.59 AM.png (2×3 px, 796 KB)

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

ginsu requested review of this revision.Mon, Jun 24, 9:45 PM
This revision is now accepted and ready to land.Thu, Jun 27, 6:29 AM

Just some nits

web/components/dropdown.react.js
33–35 ↗(On Diff #41671)

Shorthand

97–101 ↗(On Diff #41671)

I'd prefer this, but don't feel too strongly

  1. dropdownList is now positioned with absolute rather than relative. The dropdown list was causing the modal to change size whenever the dropdown was opened or closed. With the dropdownList using absolute position, the dropdown list won't affect the size of the modal. One thing to point out is now there could be overflow with the dropdown list; however this seems to be an acceptable pattern in other places that use a dropdown component

Thanks for fixing this!

Removing this diff from the stack so that I can land it now

This revision was landed with ongoing or failed builds.Fri, Jun 28, 12:00 PM
This revision was automatically updated to reflect the committed changes.