Skip to content

feat(spindle-ui): add form components#9

Merged
herablog merged 2 commits into
mainfrom
feature/ui-form
Sep 28, 2020
Merged

feat(spindle-ui): add form components#9
herablog merged 2 commits into
mainfrom
feature/ui-form

Conversation

@herablog

@herablog herablog commented Aug 12, 2020

Copy link
Copy Markdown
Member

認証プロジェクトを参考にForm関連要素のText Field, Label, Checkbox, Radio, InavalidMessageコンポーネントを作成しました。いくつか気になるところ、相談したいところがあるので、それぞれ個別にコメントします。

Storybook

Form要素のStorybook

TODO

別のPRで対応

@herablog herablog requested a review from tokimari August 12, 2020 07:30
@herablog herablog self-assigned this Aug 12, 2020

.spui-Checkbox-input:focus ~ .spui-Checkbox-outline {
/* TODO: set module background color instead of white */
box-shadow: 0 0 0 2px white, 0 0 0 4px var(--color-focus-clarity);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

outline用の1つめのbox-shadow色が、親要素の背景色(認証だと--app-surface-color)を入れるようになっているのだけど、いろんなアプリケーションで使うUIだと毎回指定ちと大変かなと思いつつ、いい方法あるかな?

  1. カスタムプロパティにして指定できるようにする
  2. outlineの実装方法を変更する (同時にスタイルも変わってしまう可能性もある)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

たしかにレイアウトによっては、Surface Colorを2枚以上重ねたり、Surface Colorに半透明の色(gray alpha 5とか)を使っている(※)要素の上にこういったComponentを配置した場合、今の outline用の1つめのbox-shadow色 が必要な実装だと、必ずしもこのoutline用の1つめのbox-shadow色 がSurface Colorと同色にならないんですよね。

(※この図のSecondary, Tertiaryを使う時
(認証の場合偶然white 100だったので問題なかったですが。)

  1. カスタムプロパティにして指定できるようにする

こっちなら配置する場所のSurface Colorに合わせて色を近づけることはできそうです。(Surface Colorが半透明色の場合、厳密に同じ色ではないですが、大きな問題にはならなそう?)

  1. outlineの実装方法を変更する (同時にスタイルも変わってしまう可能性もある)

オブジェクトの形に沿ったアウトライン形状にしたいというオーダーでbox-shadowを選択したので、outlineプロパティでは難しそう・・:thinking:

1が良さそうかなと思いつつ、他に実現方法を @hiloki 先生が知っていたりしないかな・・:eyes:チラッ

@hiloki hiloki Aug 17, 2020

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@herablog

アウトラインの内側に2pxのoffsetがあればいいってことですよね?
かつアウトライン用のspanがあるということであれば、

.spui-Checkbox-outline {
  border-radius: 2px;
  position: absolute;
  top: -2px;
  left: -2px;
  right: -2px;
  bottom: -2px;
  /* ~padding: 2px;~ さっき書いたけどこれは不要だった */
}

.spui-Checkbox-input:focus ~ .spui-Checkbox-outline {
  box-shadow: 0 0 0 2px var(--color-focus-clarity);
}

Sample: https://jsbin.com/beraqof/1/edit?html,css,output

こういう感じで実現できる予感。

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

おっ、たしかにきれいにスタイル当たる!! (背景違いでも)

Screen Shot 2020-08-18 at 10 55 54 AM

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

こちらで適用しました〜 @hiloki ありがとう:clap:
53da7b7

@herablog herablog changed the base branch from master to main August 28, 2020 06:20
@herablog herablog changed the title [WIP] feat(spindle-ui): add form components feat(spindle-ui): add form components Sep 25, 2020
@hiloki

hiloki commented Sep 25, 2020

Copy link
Copy Markdown

@herablog 複数のCheckboxやRadioで、tab操作のときにフォーカス順がおかしいのってなんでだろ?
(e.g: 1つ目のradioにfocusした後、radioのグループに遷移してしまう)

form

@herablog

Copy link
Copy Markdown
Member Author

@hiloki むむ、こりはTabのみで遷移した場合ってことですかな!?

@hiloki

hiloki commented Sep 28, 2020

Copy link
Copy Markdown

@herablog そうです!っておもったけど、radioだとその挙動がデフォルトだった...なんでもないです!

@herablog

Copy link
Copy Markdown
Member Author

@hiloki ダヨネ。確認ありがとです!

@hiloki hiloki requested review from hiloki and removed request for tokimari September 28, 2020 08:23

@hiloki hiloki left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

アイコンを含めた装飾やレイアウトはまだ未確定のところがあるので、本PRはApproveます

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants