feat(primitive): add select component #78

tarnas14 · 2022-04-20T09:27:22Z

Description

select with floating options
using theme from Provider
allows dark background (by prop)
basic tests for initial render
fix jsdoc
change to invertedStyle (after feat: main layout #64 is merged)

Motivation and Context

How Has This Been Tested?

run and tested manually

tomaszantas

Looks good! I left few comments, but most of them is related to the inconsistency between Prettier and Eslint. Fyi, I'm working on VSCode with eslint and prettier plugins, so Prettier fixes my code whenever I save the file.

tomaszantas · 2022-04-21T09:56:57Z

applications/launchpad_v2/src/components/Select/Select.test.tsx

+
+  // when
+  await act(async () => {
+    render(<ThemeProvider theme={themes.light}><Select label="Test select label" value={options[0]} options={options} onChange={() => null} /></ThemeProvider>)


This line seems to be too long (around 160 characters long). Prettier should try to wrap this. I found that eslint won't complain on this. Maybe we should add max-len rule to eslint config (as warning)? What do you think? Prettier uses on 80 characters limit.

I connected prettier to eslint and ran lint:fix, it should be good now

tomaszantas · 2022-04-21T09:58:33Z

applications/launchpad_v2/src/components/Select/types.ts

+}
+
+type Option = { value: string; label: string; key: string; }
+export type MyListboxProps = {


Suggested change

export type MyListboxProps = {

export type SelectProps = {

tomaszantas · 2022-04-21T09:58:54Z

applications/launchpad_v2/src/components/Select/index.tsx

+import ArrowBottom from '../../styles/Icons/ArrowBottom1'
+
+import { Label, SelectButton, SelectorIcon, OptionsContainer, Option } from './styles'
+import { MyListboxProps } from './types'


Suggested change

import { MyListboxProps } from './types'

import { SelectProps } from './types'

tomaszantas · 2022-04-21T09:59:05Z

applications/launchpad_v2/src/components/Select/index.tsx

+ * @prop {string} label - label shown in option
+ * @prop {string} key - key to be used in react map
+ */
+const Select = ({ value, options, onChange, inverted, label }: MyListboxProps) => {


Suggested change

const Select = ({ value, options, onChange, inverted, label }: MyListboxProps) => {

const Select = ({ value, options, onChange, inverted, label }: SelectProps) => {

tomaszantas · 2022-04-21T10:03:20Z

applications/launchpad_v2/src/containers/BaseNodeContainer/index.tsx

+        />
+      </div>
+
+


Here are 2 empty lines - another inconsistency between prettier and eslint. Maybe we should add no-multiple-empty-lines rule?

I connected prettier to eslint and ran lint:fix, it should be good now

tomaszantas · 2022-04-21T10:09:41Z

applications/launchpad_v2/src/styles/themes/index.ts

@@ -1,9 +1,16 @@
 import darkTheme from './dark'
 import lightTheme from './light'

+const withShared = theme => ({
+  ...theme,
+  borderRadius: (count = 1) => `${count * 10}px`,


Cool! Just curious: how did you pick up the base values (10px, 0.7px and 1.3px)?

pure guess
looked good, but I thought this way it will be easier to adjust them later ^^

tomaszantas

LGTM! You can switch to describe-it in the Select.test.tsx. But it's minor, and eventually can be changed later to not block this PR.

tomaszantas · 2022-04-22T10:05:52Z

applications/launchpad_v2/src/components/Select/Select.test.tsx

+import themes from '../../styles/themes'
+import Select from './'
+
+test('renders label with select', async () => {


NIT: We used to:

describe('Component name', () => { it('should render X', () => {}) })

will do on next branch ,thanks

tarnas14 added 9 commits April 20, 2022 11:03

quick and dirty one-file dropdown

48ac83a

added label to select

87798c0

extracted value and options and made it configurable in the select

57bbead

extract Select to a reusable dumb component

c14571d

refactor WithTheme HoC to correctly set displayName

7ae4247

extract transparent background to theme

7cd67e6

fix types for styledComponents in Select

0462f22

add basic tests to Select component

5151138

typescript children: ReactNode typing

d6b9ed8

tarnas14 changed the title ~~feat(primitive): add select component~~ [WIP] feat(primitive): add select component Apr 20, 2022

tarnas14 added 8 commits April 20, 2022 12:56

change darkBackground prop name to inverted

a8d7e39

use default export in Select component

c919134

move type declarations to separate types.ts files

9eb0add

fix test to use correct import

7360a18

Merge branch 'launchpad_such_wow' into launchpad/feature/63/select

ab10f2e

add jsdoc to Select component

e714635

improve jsdoc

a17693b

Merge branch 'launchpad_such_wow' into launchpad/feature/63/select

c4c75e7

tarnas14 changed the title ~~[WIP] feat(primitive): add select component~~ feat(primitive): add select component Apr 21, 2022

tarnas14 changed the title ~~feat(primitive): add select component~~ [wip] feat(primitive): add select component Apr 21, 2022

tarnas14 marked this pull request as draft April 21, 2022 08:48

tarnas14 changed the title ~~[wip] feat(primitive): add select component~~ feat(primitive): add select component Apr 21, 2022

tarnas14 added 2 commits April 21, 2022 11:12

improved styling with the inverted values

0befb4c

fix Select tests - pass correct theme

65098e6

tarnas14 marked this pull request as ready for review April 21, 2022 09:18

tarnas14 added 2 commits April 21, 2022 11:22

remove last darkBackground reference

94cc11f

avoid using optional chaining to not angry CI gods

311f194

tomaszantas requested changes Apr 21, 2022

View reviewed changes

tarnas14 added 2 commits April 21, 2022 19:25

hooked up prettier to eslint and fixed all

0d0baa0

rename MyListboxProps to SelectProps

7dd43d7

tarnas14 added 2 commits April 21, 2022 19:31

Merge branch 'launchpad_such_wow' into launchpad/feature/63/select

e8d7856

lint fix with prettier after merge

d502ef7

tomaszantas approved these changes Apr 22, 2022

View reviewed changes

tarnas14 merged commit 8417cb9 into launchpad_such_wow Apr 22, 2022

tarnas14 deleted the launchpad/feature/63/select branch April 22, 2022 10:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(primitive): add select component #78

feat(primitive): add select component #78

tarnas14 commented Apr 20, 2022 •

edited

Loading

tomaszantas left a comment

tomaszantas Apr 21, 2022

tarnas14 Apr 21, 2022

tomaszantas Apr 21, 2022

tarnas14 Apr 21, 2022

tomaszantas Apr 21, 2022

tarnas14 Apr 21, 2022

tomaszantas Apr 21, 2022

tomaszantas Apr 21, 2022

tarnas14 Apr 21, 2022

tomaszantas Apr 21, 2022

tarnas14 Apr 21, 2022

tomaszantas left a comment

tomaszantas Apr 22, 2022

tarnas14 Apr 22, 2022

	import { MyListboxProps } from './types'
	import { SelectProps } from './types'

	const Select = ({ value, options, onChange, inverted, label }: MyListboxProps) => {
	const Select = ({ value, options, onChange, inverted, label }: SelectProps) => {

feat(primitive): add select component #78

feat(primitive): add select component #78

Conversation

tarnas14 commented Apr 20, 2022 • edited Loading

Description

Motivation and Context

How Has This Been Tested?

tomaszantas left a 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tomaszantas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tarnas14 commented Apr 20, 2022 •

edited

Loading