feat: bunch of improvements

onfido · Sep 10, 2021 · ad64066 · ad64066
1 parent 3c871c9
commit ad64066
Show file tree

Hide file tree

Showing 11 changed files with 127 additions and 104 deletions.
diff --git a/packages/core/src/components/select/select.scss b/packages/core/src/components/select/select.scss
@@ -18,8 +18,9 @@
     display: inline-flex;
     flex-direction: column;
     justify-content: center;
+    max-width: helpers.space(50);
     position: relative;
-    width: helpers.space(50);
+    width: 100%;
 
     > * {
       box-sizing: inherit;
@@ -29,7 +30,7 @@
       width: fit-content;
     }
 
-    &.-empty {
+    &.-empty > * {
       color: helpers.color('content-placeholder');
     }
 
@@ -45,29 +46,29 @@
   .ods-select-native {
     @include mixins.as-text-field();
     @include mixins.interactive();
+    @include display();
     @include with-padding(1px); // border width set on `as-text-field` mixin
 
     appearance: none; // removes native arrow that opens the select
 
-    + .ods-icon {
-      color: helpers.color('content-main');
+    &.-absolute {
+      position: absolute;
+    }
+
+    ~ .ods-icon {
       pointer-events: none;
       position: absolute;
       right: helpers.space(2);
     }
 
-    &.-invalid,
-    &.-touched:invalid,
-    &.-invalid + .ods-icon,
-    &.-touched:invalid + .ods-icon,
-    &.-invalid ~ .ods-select-output,
-    &.-touched:invalid ~ .ods-select-output {
+    &.-invalid ~ *,
+    &.-touched:invalid ~ * {
       color: helpers.color('content-negative');
     }
 
     &:disabled {
       &,
-      + .ods-icon {
+      ~ * {
         color: helpers.color('content-disabled');
       }
     }
@@ -96,13 +97,9 @@
 
   .ods-select-output {
     @include helpers.font('300-regular');
+    @include display();
     @include with-padding();
-    margin-top: helpers.space(-6);
-    overflow: hidden;
     pointer-events: none;
-    text-overflow: ellipsis;
-    white-space: nowrap;
-    width: 100%;
   }
 
   .ods-select-dropdown {
@@ -168,6 +165,13 @@
   }
 }
 
+@mixin display() {
+  overflow: hidden;
+  position: relative;
+  text-overflow: ellipsis;
+  white-space: nowrap;
+}
+
 @mixin with-padding($border-width: 0) {
   @if type-of($border-width) != number {
     @error '$border-width must be a number';

diff --git a/packages/core/src/mixins/as-text-field.scss b/packages/core/src/mixins/as-text-field.scss
@@ -11,6 +11,7 @@
   border-radius: helpers.border-radius('medium');
   box-sizing: border-box;
   color: helpers.color('content-main');
+  opacity: 1;
   padding: helpers.space(1.5) - $border-width helpers.space(2) - $border-width;
   width: 100%;
 

diff --git a/packages/react/src/components/select/custom/custom-option-group.tsx b/packages/react/src/components/select/custom/custom-option-group.tsx
@@ -1,16 +1,18 @@
 import React from 'react';
 
+export interface CustomOptionGroupProps extends JsxDiv {
+  label?: string;
+}
+
 export const CustomOptionGroup = ({
   children,
   label,
   ...props
-}: CustomOptionGroupProps): JSX.Element => (
+}: CustomOptionGroupProps) => (
   <div {...props}>
     {label}
     {children}
   </div>
 );
 
-export type CustomOptionGroupProps = JSX.IntrinsicElements['div'] & {
-  label?: string;
-};
+type JsxDiv = JSX.IntrinsicElements['div'];
diff --git a/packages/react/src/components/select/custom/custom-option.tsx b/packages/react/src/components/select/custom/custom-option.tsx
@@ -2,33 +2,41 @@ import { c, classy } from '@onfido/castor';
 import React, { ReactNode, useEffect } from 'react';
 import { useCustomSelect } from './useCustomSelect';
 
+export interface CustomOptionProps extends JsxLabel {
+  children?: ReactNode;
+  disabled?: boolean;
+  value: string | number | readonly string[];
+}
+
 export function CustomOption({
   children,
   disabled,
   value: optionValue,
+  onKeyUp,
+  onMouseUp,
   ...props
 }: CustomOptionProps) {
-  const { name, select, selectedOption, value } = useCustomSelect();
+  const { name, register, select, value } = useCustomSelect();
 
-  // defaultValue check
-  useEffect(() => {
-    if (selected && !selectedOption) select(children);
-  }, []);
+  useEffect(() => register(children, optionValue), []);
 
-  const selected = value == optionValue;
   const selectOption = () => select(children, optionValue);
 
   return (
     <label
       {...props}
       className={classy(c('select-option'))}
-      onKeyUp={({ key }) => {
-        if (selectOptionKeys.has(key)) selectOption();
+      onKeyUp={(event) => {
+        if (selectOptionKeys.has(event.key)) selectOption();
+        onKeyUp?.(event);
+      }}
+      onMouseUp={(event) => {
+        selectOption();
+        onMouseUp?.(event);
       }}
-      onMouseUp={selectOption}
     >
       <input
-        checked={selected}
+        checked={value == optionValue}
         disabled={disabled}
         name={name}
         readOnly
@@ -39,10 +47,6 @@ export function CustomOption({
   );
 }
 
-export type CustomOptionProps = JSX.IntrinsicElements['label'] & {
-  children?: ReactNode;
-  disabled?: boolean;
-  value: string | number | readonly string[];
-};
-
 const selectOptionKeys = new Set([' ', 'Enter']);
+
+type JsxLabel = JSX.IntrinsicElements['label'];
diff --git a/packages/react/src/components/select/custom/custom-select.tsx b/packages/react/src/components/select/custom/custom-select.tsx
@@ -33,62 +33,68 @@ export function CustomSelect({
     <CustomSelectProvider
       value={{
         name: name || `castor-select-${++id}`,
+        register(option, optionValue) {
+          // initial value
+          if (value == optionValue) setSelectedOption(option);
+          // or placeholder is first option
+          else setSelectedOption((current) => current ?? option);
+        },
         select(option, value) {
           setSelectedOption(option);
           setValue(value);
-          // there is no value on defaultValue assignments, so don't close
-          if (value != null) close();
+          close();
         },
         selectedOption,
         value,
       }}
     >
       <NativeSelect
         {...props}
-        childrenAreNotOptions
+        notNative
         ref={selectRef}
         className={className}
         name={name}
         open={isOpen}
         value={value}
         onClick={() => (isOpen ? close() : open())}
-        onKeyUp={(ev) => {
-          if (openSelectKeys.has(ev.key)) open();
+        onKeyUp={(event) => {
+          if (openSelectKeys.has(event.key)) open();
         }}
       >
         <output className={classy(c('select-output'))}>{selectedOption}</output>
-        {isOpen && (
-          <Popover
-            align={align}
-            className={classy(c('select-dropdown'))}
-            position={position}
-            target={selectRef}
-            onClose={() => setIsOpen(false)}
-            onKeyUp={(ev) => {
-              if (closeSelectKeys.has(ev.key)) close();
-            }}
-            onKeyDown={(ev) => {
-              // close if focus moves outside
-              if (ev.key === 'Tab') {
-                setTimeout(close);
-                ev.preventDefault();
-              }
-            }}
-            onRender={(element) =>
-              // focus either selected or first question
-              focus(
-                element?.querySelector(':checked') ??
-                  element?.querySelector('input')
-              )
+      </NativeSelect>
+
+      {isOpen && (
+        <Popover
+          align={align}
+          className={classy(c('select-dropdown'))}
+          position={position}
+          target={selectRef}
+          onClose={() => setIsOpen(false)}
+          onKeyUp={(ev) => {
+            if (closeSelectKeys.has(ev.key)) close();
+          }}
+          onKeyDown={(event) => {
+            // close if focus moves outside
+            if (event.key === 'Tab') {
+              setTimeout(close);
+              event.preventDefault();
             }
-          >
-            {children}
-          </Popover>
-        )}
+          }}
+          onRender={(element) =>
+            // focus either selected or first question
+            focus(
+              element?.querySelector(':checked') ??
+                element?.querySelector('input')
+            )
+          }
+        >
+          {children}
+        </Popover>
+      )}
 
-        {/* render once to find placeholder */}
-        {!selectedOption && <div style={{ display: 'none' }}>{children}</div>}
-      </NativeSelect>
+      {/* render once to find placeholder */}
+      {!selectedOption && <div style={{ display: 'none' }}>{children}</div>}
     </CustomSelectProvider>
   );
 }

diff --git a/packages/react/src/components/select/custom/useCustomSelect.ts b/packages/react/src/components/select/custom/useCustomSelect.ts
@@ -8,9 +8,10 @@ export const CustomSelectProvider = CustomSelectContext.Provider;
 
 export interface CustomSelectState {
   name?: string;
-  select: (option: ReactNode, value?: Value) => void;
+  register: (option: ReactNode, value: Value) => void;
+  select: (option: ReactNode, value: Value) => void;
   selectedOption?: ReactNode;
   value?: Value;
 }
 
-type Value = JSX.IntrinsicElements['select']['value'];
+type Value = string | number | readonly string[];
diff --git a/packages/react/src/components/select/native/native-option-group.tsx b/packages/react/src/components/select/native/native-option-group.tsx
@@ -1,7 +1,7 @@
 import React from 'react';
 
-export const NativeOptionGroup = (
-  props: NativeOptionGroupProps
-): JSX.Element => <optgroup {...props} />;
-
 export type NativeOptionGroupProps = JSX.IntrinsicElements['optgroup'];
+
+export const NativeOptionGroup = (props: NativeOptionGroupProps) => (
+  <optgroup {...props} />
+);
diff --git a/packages/react/src/components/select/native/native-option.tsx b/packages/react/src/components/select/native/native-option.tsx
@@ -1,7 +1,5 @@
 import React from 'react';
 
-export const NativeOption = (props: NativeOptionProps): JSX.Element => (
-  <option {...props} />
-);
-
 export type NativeOptionProps = JSX.IntrinsicElements['option'];
+
+export const NativeOption = (props: NativeOptionProps) => <option {...props} />;
diff --git a/packages/react/src/components/select/native/native-select.tsx b/packages/react/src/components/select/native/native-select.tsx
@@ -4,7 +4,7 @@ import React, { useEffect, useState } from 'react';
 import { withRef } from '../../../utils';
 
 export interface NativeSelectProps extends BaseProps, JsxSelect {
-  childrenAreNotOptions?: boolean;
+  notNative?: boolean;
   open?: boolean;
 }
 
@@ -13,9 +13,9 @@ export const NativeSelect = withRef(function NativeSelect(
     id = `castor_native_select_${++idCount}`,
     borderless,
     children,
-    childrenAreNotOptions,
     className,
     invalid,
+    notNative,
     onChange,
     open,
     ...restProps
@@ -37,16 +37,19 @@ export const NativeSelect = withRef(function NativeSelect(
         {...restProps}
         ref={ref}
         id={id}
-        className={classy(c('select-native'), m({ invalid, touched }))}
+        className={classy(
+          c('select-native'),
+          m({ absolute: notNative, invalid, touched })
+        )}
         onChange={(event) => {
           setEmpty(!event.currentTarget.value);
           onChange?.(event);
         }}
       >
-        {!childrenAreNotOptions && children}
+        {notNative ? <option hidden value={value}></option> : children}
       </select>
+      {notNative && children}
       <Icon name="chevron-down" aria-hidden="true" />
-      {childrenAreNotOptions && children}
     </div>
   );
 });