frontend: style multi select dropdown #2129

shonsirsha · 2023-05-29T17:20:57Z

to improve our UI, we'd like to style our Active Currencies multi dropdown.

In this PR we also refactored it, along with the single dropdown component.

Most up-to-date preview:

Screen.Recording.2023-05-30.at.15.55.50.mov

shonsirsha · 2023-05-30T08:43:47Z

There's unfortunately a known issue in react-select that I discovered while developing. When opening the multi select/dropdown, it automatically focuses on the first option. I believe this shouldn't be a default behaviour, but unfort. it is, and I still have no way to fix it that's not too hacky.

(right now, only the focus colour is not shown due to CSS. But technically/programatically, it's still focusing automatically on the first option - so if a user opens the dropdown and presses the return key, they'll select the first option).

SO question: https://stackoverflow.com/questions/65510875/disable-focus-on-first-option-in-react-select (the solution is outdated as it's intended for v4, we're using v5).

Github PR that's been open for years at their end and also some discussion: JedWatson/react-select#4080

thisconnect · 2023-05-30T09:37:06Z

There's unfortunately a known issue in react-select that I discovered while developing. When opening the multi select/dropdown, it automatically focuses on the first option. I believe this shouldn't be a default behaviour, but unfort. it is, and I still have no way to fix it that's not too hacky.

IMO having focus on the first item is fine. but the focus should be visible.

shonsirsha · 2023-05-30T10:06:26Z

frontends/web/src/routes/new-settings/components/dropdowns/activecurrenciesdropdown.module.css

+/*
+displays ', ' as a pseudo component for all currency component
+except the last displayed component (the 3rd currency component).
+
+The reason for :nth-last-child(2) instead of just :last-child is because there's an extra
+component created by react-select after the actual last currency component. 
+
+So, we're targetting the second to last component here, 
+which is the last selected currency in the DOM.
+*/
+.select:not(.hideMultiSelect) :global(.react-select__multi-value):not(:nth-last-child(2))::after {
+    content: ', ';
+}
+
+/*
+displays '...' as a pseudo component of the 3rd currency and only when there's more than 3 selected.
+:nth-last-child(2) is used for the same reason as above
+*/
+.select:not(.hideMultiSelect) :global(.react-select__multi-value):nth-child(3):not(:nth-last-child(2))::after {
+    content: ' ...';
+}
+


Might look "hacky" here just to display a string like "GBP, CHF" or "GBP, CHF, USD" or "AUD, GBP, CHF ..."

but for multi select using react-select, we can't just display a single string unfortunately - as AFAIK it's designed to display each selected item as a single component.

Under the hood, in react-select multi, for each selected component, a single component (div) with class .react-select__multi-value gets generated and shown. Here, we just separate them with ',' and then '...' for the last element if there's more than 3 selected.

We can omit this pseudo element thing if it looks too hacky, but only if we want to re-design the UI of that part a little bit... To be something like this 😄

we ofc can remove the "X", and just have it as "badges", the way they're intended to be rendered - at least according to the lib.

the "badges" with close button look cool, but I guess not necessary I guess better if we keep it minimal text-only for now.

shonsirsha · 2023-05-30T10:08:36Z

frontends/web/src/routes/new-settings/components/dropdowns/activecurrenciesdropdown.tsx

+      isMulti
+      closeMenuOnSelect={false}
+      hideSelectedOptions={false}
+      value={[...selectedCurrencies].reverse()}


reverse() here so the latest selected currency will be displayed at the most front of the string.

E.g:

Let's say selected currencies looks like this: GBP, EUR

User selects AUD

Now, selected looks like this: AUD, GBP, EUR

ahh that makes sense, I was wondering what the order is and why it changes before. This way it always shows the newest nice 👍

shonsirsha · 2023-05-30T11:46:01Z

changes made, demo updated in PR's desc.
PTAL 🙏

cc @jadzeidan

thisconnect · 2023-06-06T10:58:41Z

frontends/web/src/api/account.ts

+export type FiatWithDisplayName = {
+  currency: Fiat,
+  displayName: string
+}


Since this is only used in rates component and not coming from backend I think better to move to rates.tsx

thisconnect · 2023-06-06T11:00:15Z

frontends/web/src/components/icon/icon.tsx

 import saveLightSVG from './assets/icons/save-light.svg';
 import starSVG from './assets/icons/star.svg';
 import starInactiveSVG from './assets/icons/star-inactive.svg';
+import selectedCheckLight from './assets/icons/selected-check-light.svg';


nit: for consistency add SVG at the end as all other imported svgs end with SVG in the imoported name.

thisconnect · 2023-06-06T11:00:49Z

frontends/web/src/components/icon/icon.tsx

 export const SaveLight = (props: ImgProps) => (<img src={saveLightSVG} draggable={false} {...props} />);
 export const Star = (props: ImgProps) => (<img src={starSVG} draggable={false} {...props} />);
 export const StarInactive = (props: ImgProps) => (<img src={starInactiveSVG} draggable={false} {...props} />);
+// check component for cusotm multi-dropdown select


comment not needed IMO, this icon could also be used in a different component in the future.

thisconnect · 2023-06-06T11:07:19Z

frontends/web/src/components/rates/rates.tsx

+  { currency: 'SEK', displayName: 'Swedish krona' },
+  { currency: 'SGD', displayName: 'Singapore dollar' },
+  { currency: 'USD', displayName: 'United States dollar' },
+  { currency: 'BTC', displayName: 'Bitcoin' }


I thought about if we should display the currency name in the native language. i.e. "Svensk krona" instead of "Swedish krona", but that does't really work for "Swiss france" with our 4 official languages 🤦

so I guess better to have everything in English.

Please note we have a new lang (Czech) and new Fiat currencies on master, maybe merge master->staging-revamp-settings first?

#2141

thisconnect · 2023-06-06T11:23:17Z

frontends/web/src/routes/new-settings/components/appearance/activeCurrenciesDropdownSetting.tsx


 const ActiveCurrenciesDropdownSetting = ({ selected, active }: SharedProps) => {
-  const formattedCurrencies = currencies.map((currency) => ({ label: currency, value: currency }));
+  const formattedCurrencies = currenciesWithDisplayName.map((fiat) => ({ label: `${fiat.displayName} (${fiat.currency})`, value: fiat.currency }));


nit: this calls array map over and over but the formattedCurrencies are always the same. Maybe rates component could map once and export?

same with formattedCurrencies in defaultCurrencyDropdownSetting component (as it now always contains all currencies)

Sure I'll do this, but these exported values are probably only gonna be used in the respective dropdown(s), as it's only used for exactly the dropdown/select component (thus it needs a label and value).

Will also move defaultValueLabel 😉 👓

thisconnect · 2023-06-06T11:33:16Z

frontends/web/src/routes/new-settings/components/dropdowns/activecurrenciesdropdown.module.css

+which is the last selected currency in the DOM.
+*/
+.select:not(.hideMultiSelect) :global(.react-select__multi-value):not(:nth-last-child(2))::after {
+    content: ', ';


looks like space doens't work it seems to collapse and be ignored in pseudo element via content.

It would look nicer with a bit of space after the comma, can you add padding-right: 2px;?

Ooh lol good catch, I didnt even notice.. 🤔

thisconnect · 2023-06-06T11:40:21Z

frontends/web/src/routes/new-settings/components/dropdowns/activecurrenciesdropdown.module.css

+:nth-last-child(2) is used for the same reason as above
+*/
+.select:not(.hideMultiSelect) :global(.react-select__multi-value):nth-child(3):not(:nth-last-child(2))::after {
+    content: ' ...';


This CSS selector is a bit crazy :) but I guess adding … via JSX would make the component a lot more complicated. then fine I guess…

remove space as it doesn't support leading space in content property and I think space is not needed from layout perspective.

and could you use … tripple dot … 🤓 ?

thisconnect · 2023-06-06T11:42:57Z

frontends/web/src/routes/new-settings/components/dropdowns/activecurrenciesdropdown.module.css

+/*
+displays ', ' as a pseudo component for all currency component
+except the last displayed component (the 3rd currency component).
+
+The reason for :nth-last-child(2) instead of just :last-child is because there's an extra
+component created by react-select after the actual last currency component. 
+
+So, we're targetting the second to last component here, 
+which is the last selected currency in the DOM.
+*/
+.select:not(.hideMultiSelect) :global(.react-select__multi-value):not(:nth-last-child(2))::after {
+    content: ', ';
+}
+
+/*
+displays '...' as a pseudo component of the 3rd currency and only when there's more than 3 selected.
+:nth-last-child(2) is used for the same reason as above
+*/
+.select:not(.hideMultiSelect) :global(.react-select__multi-value):nth-child(3):not(:nth-last-child(2))::after {
+    content: ' ...';
+}
+


the "badges" with close button look cool, but I guess not necessary I guess better if we keep it minimal text-only for now.

thisconnect · 2023-06-06T11:45:40Z

frontends/web/src/routes/new-settings/components/dropdowns/activecurrenciesdropdown.tsx

+      isMulti
+      closeMenuOnSelect={false}
+      hideSelectedOptions={false}
+      value={[...selectedCurrencies].reverse()}


ahh that makes sense, I was wondering what the order is and why it changes before. This way it always shows the newest nice 👍

thisconnect · 2023-06-06T11:53:02Z

frontends/web/src/routes/new-settings/components/dropdowns/singledropdown.tsx

-import Select, { components, DropdownIndicatorProps, SingleValue as SingleValueType, SingleValueProps, OptionProps } from 'react-select';
-import styles from './singledropdown.module.css';
+import Select, { components, DropdownIndicatorProps, SingleValueProps, OptionProps } from 'react-select';
+import dropdownStyles from '../dropdowns/dropdowns.module.css';


from './dropdowns.module.css';

to improve our UI, we'd like to style our Active Currencies multi dropdown. In this commit we also refactored it, along with the single dropdown component.

shonsirsha · 2023-06-06T17:43:56Z

@thisconnect Thanks for the review! Changes made, PTAL 🙏

Also... yeah why not.. we can merge master to the staging branch first 🤷‍♂️
Here's the PR master->staging-revamp-settings: #2142

thisconnect

thanks LGTM

…ect-dropdown-active-currencies

shonsirsha marked this pull request as draft May 29, 2023 17:21

shonsirsha changed the title ~~frontend: WIP custom multi select dropdown~~ [draft] frontend: WIP custom multi select dropdown May 29, 2023

shonsirsha force-pushed the frontend-custom-multi-select-dropdown-active-currencies branch 2 times, most recently from c443eff to d4d4261 Compare May 30, 2023 08:01

shonsirsha changed the title ~~[draft] frontend: WIP custom multi select dropdown~~ frontend: WIP custom multi select dropdown May 30, 2023

shonsirsha marked this pull request as ready for review May 30, 2023 08:29

shonsirsha changed the title ~~frontend: WIP custom multi select dropdown~~ frontend: style multi select dropdown May 30, 2023

shonsirsha commented May 30, 2023

View reviewed changes

shonsirsha force-pushed the frontend-custom-multi-select-dropdown-active-currencies branch 2 times, most recently from c76b834 to 325775d Compare May 30, 2023 11:42

shonsirsha force-pushed the frontend-custom-multi-select-dropdown-active-currencies branch 3 times, most recently from fe00852 to ba438c6 Compare May 30, 2023 14:02

shonsirsha requested a review from thisconnect May 31, 2023 19:28

thisconnect requested changes Jun 6, 2023

View reviewed changes

frontend: styled multi select for Active Currencies Dropdown / Select

9a8bd51

to improve our UI, we'd like to style our Active Currencies multi dropdown. In this commit we also refactored it, along with the single dropdown component.

shonsirsha force-pushed the frontend-custom-multi-select-dropdown-active-currencies branch from ba438c6 to 9a8bd51 Compare June 6, 2023 17:41

shonsirsha mentioned this pull request Jun 6, 2023

Master to staging-revamp-settings #2142

Merged

thisconnect approved these changes Jun 6, 2023

View reviewed changes

Merge branch 'staging-revamp-settings' into frontend-custom-multi-sel…

208f772

…ect-dropdown-active-currencies

shonsirsha merged commit 2dd42f4 into BitBoxSwiss:staging-revamp-settings Jun 11, 2023

frontend: style multi select dropdown #2129

frontend: style multi select dropdown #2129

Uh oh!

Conversation

shonsirsha commented May 29, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

shonsirsha commented May 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

thisconnect commented May 30, 2023

Uh oh!

shonsirsha May 30, 2023 • 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!

shonsirsha May 30, 2023 • 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!

shonsirsha commented May 30, 2023

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!

thisconnect Jun 6, 2023 • 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!

shonsirsha Jun 6, 2023 • 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!

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!

shonsirsha commented Jun 6, 2023

Uh oh!

thisconnect left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

shonsirsha commented May 29, 2023 •

edited

Loading

shonsirsha commented May 30, 2023 •

edited

Loading

shonsirsha May 30, 2023 •

edited

Loading

shonsirsha May 30, 2023 •

edited

Loading

thisconnect Jun 6, 2023 •

edited

Loading

shonsirsha Jun 6, 2023 •

edited

Loading