Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix input disabled style #1636

Merged
merged 13 commits into from
Jun 24, 2021
Merged

Fix input disabled style #1636

merged 13 commits into from
Jun 24, 2021

Conversation

AtsushiM
Copy link
Member

@AtsushiM AtsushiM commented Jun 1, 2021 •
edited
Loading

Related URL

Overview

  • 入力要素のスタイルがブラウザによって差分が発生しているので、揃えたい

What I did

  • Input, Select, Textarea にstyleを設定
    • 確認しやすいよう、Comboboxのstorybookにコード追加していますが、LGTM出たら消します。

Capture

IE11

スクリーンショット 2021-06-14 12 35 52

Edge

スクリーンショット 2021-06-14 12 38 42

Mobile Safari

スクリーンショット 2021-06-14 12 36 03

Chrome

スクリーンショット 2021-06-14 12 36 20

Firefox

スクリーンショット 2021-06-14 12 37 41

@AtsushiM AtsushiM requested a review from a team as a code owner June 1, 2021 00:38
@AtsushiM AtsushiM requested review from moshisora and Tokky0425 and removed request for a team June 1, 2021 00:38
@netlify
Copy link

netlify bot commented Jun 1, 2021 •
edited
Loading

✔️ Deploy Preview for smarthr-ui ready!

🔨 Explore the source changes: 56966cf

🔍 Inspect the deploy log: https://app.netlify.com/sites/smarthr-ui/deploys/60d28e44036a400008ed7039

😎 Browse the preview: https://deploy-preview-1636--smarthr-ui.netlify.app/iframe

@AtsushiM AtsushiM force-pushed the fix-input-disabled-style branch from d2b2f9a to ccc985e Compare June 14, 2021 03:54
@AtsushiM AtsushiM changed the title [WIP] Fix input disabled style Fix input disabled style Jun 14, 2021
wmoai
wmoai reviewed Jun 21, 2021
View reviewed changes
Copy link
Contributor

@wmoai wmoai left a comment

Choose a reason for hiding this comment

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

コメントしました！

src/components/Textarea/Textarea.tsx
@@ -76,6 +76,7 @@ const StyledTextarea = styled.textarea<Props & { themes: Theme; textAreaWidth?:
background-color: #fff;
border: ${frame.border.default};
box-sizing: border-box;
opacity: 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

単純に質問なのですが、こちらは何に対処するための指定でしょうか？

Copy link
Member Author

Choose a reason for hiding this comment

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

Mobile Safariです。
53163d3

確認した環境はシュミレータですが

  • iPhone 12 Pro Max
  • iOS 14.5

となっています。

Copy link
Contributor

Choose a reason for hiding this comment

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

ありがとうございます！把握です！

src/components/ComboBox/ComboBox.stories.tsx Outdated
@@ -247,6 +251,41 @@ export const Disabled: Story = () => {
}}
/>
</dd>
<dd>
Copy link
Contributor

Choose a reason for hiding this comment

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

[IMO] こちらの比較用 story はあるととても便利だと思うのですが、 ComboBox の Disabled story 内にあるのはやや違和感がある気がしました。
個人的には、 Input コンポーネントを基準とみなして、 Input 配下に比較専用の story を新規で作成するのが良いのではないかと思いました。

Copy link
Member Author

Choose a reason for hiding this comment

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

@wmoai あ、こちら今回のPRで見やすくするために追加したもので、merge前に消す想定でした。

確認しやすいよう、Comboboxのstorybookにコード追加していますが、LGTM出たら消します。

ただ

こちらの比較用 story はあるととても便利だと思うのですが(ry

などから別途作ったほうがいいですかね？

Copy link
Contributor

Choose a reason for hiding this comment

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

あ、なるほどスミマセン。であれば消すで良いと思います！

wmoai
wmoai previously approved these changes Jun 21, 2021
View reviewed changes
Copy link
Contributor

@wmoai wmoai left a comment

Choose a reason for hiding this comment

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

LGTM! 🎉

@AtsushiM
Copy link
Member Author

@wmoai storybook再調整しました。再確認お願いできると 🙏

@AtsushiM AtsushiM force-pushed the fix-input-disabled-style branch from 39ea360 to 56966cf Compare June 23, 2021 01:28
wmoai
wmoai approved these changes Jun 24, 2021
View reviewed changes
Copy link
Contributor

@wmoai wmoai left a comment

Choose a reason for hiding this comment

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

LGTM !

@AtsushiM AtsushiM merged commit dae134b into kufu:master Jun 24, 2021
@AtsushiM AtsushiM deleted the fix-input-disabled-style branch June 24, 2021 06:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@wmoai wmoai wmoai approved these changes

@moshisora moshisora Awaiting requested review from moshisora

@Tokky0425 Tokky0425 Awaiting requested review from Tokky0425

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@AtsushiM @wmoai