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.

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

feat: add Shin color (SHRUI-227) #1141

Merged
merged 8 commits into from Nov 20, 2020
Merged

feat: add Shin color (SHRUI-227) #1141

merged 8 commits into from Nov 20, 2020

Conversation

wmoai
Copy link
Contributor

@wmoai wmoai commented Nov 11, 2020

Related URL

https://smarthr.atlassian.net/browse/SHRUI-227

Overview

新カラーパレットを追加し、使用できるようにします。

What I did

  • BASE_GREY色を追加
  • 新カラーパレットを追加
    • createTheme({ palette: shinColorPalette }) で新カラーパレットを適用可能
  • コンポーネント内でthemeカラーのコードを直書きしている部分をtheme参照に変更

やってないこと

  • storybookのstory内には現defaultPaletteのカラーコードを直書きしている部分がありますが、これらを全て修正するとなると変更が膨大になりバックポートの影響範囲が広がってしまうためこのPRでは対応は行わず、このPR後に行われるdefaultPalette置き換え対応時に合わせて対応したいと思います

Capture

スクリーンショット 2020-11-12 10 28 02

@wmoai wmoai requested a review from a team as a code owner November 11, 2020 03:38
@wmoai wmoai requested review from AtsushiM and cidermitaina and removed request for a team November 11, 2020 03:38
@reg-suit
Copy link

reg-suit bot commented Nov 11, 2020

reg-suit detected visual differences.

Check this report, and review them.

🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴🔴

🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵🔵

What do the circles mean? The number of circles represent the number of changed images.
🔴 : Changed items, ⚪ : New items, ⚫ : Deleted items, and 🔵 Passed items

How can I change the check status? If reviewers approve this PR, the reg context status will be green automatically.

@netlify
Copy link

netlify bot commented Nov 11, 2020

Deploy preview for smarthr-ui ready!

Built with commit 2c727db

https://deploy-preview-1141--smarthr-ui.netlify.app

@wmoai wmoai self-assigned this Nov 12, 2020
@wmoai wmoai changed the title [WIP] feat: add Shin color (SHRUI-227) feat: add Shin color (SHRUI-227) Nov 12, 2020
Copy link
Member

@cidermitaina cidermitaina left a comment

Choose a reason for hiding this comment

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

ありがとうございます!LGTM🎉

storybookのstory内には現defaultPaletteのカラーコードを直書きしている部分がありますが、これらを全て修正するとなると変更が膨大になりバックポートの影響範囲が広がってしまうためこのPRでは対応は行わず、このPR後に行われるdefaultPalette置き換え対応時に合わせて対応したいと思います

こちらの方針も良きと思います!🙋‍♀️

Copy link
Member

@nabeliwo nabeliwo left a comment

Choose a reason for hiding this comment

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

コード的には問題なさそうです!
この PR 自体って v11 の時点から切ったブランチに向けなくて大丈夫でしたっけ?

@wmoai
Copy link
Contributor Author

wmoai commented Nov 12, 2020

@nabeliwo このPR自体はmasterにマージして、そのマージコミットを各バージョンタグから切ったブランチにcherry-pickすることでバックポートするのが良いのではないかと考えています!

Copy link
Member

@nabeliwo nabeliwo left a comment

Choose a reason for hiding this comment

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

このPR自体はmasterにマージして、そのマージコミットを各バージョンタグから切ったブランチにcherry-pickすることでバックポートする

なるほど 💡
良さそうです!

@AtsushiM AtsushiM removed their request for review November 16, 2020 06:47
@cidermitaina cidermitaina merged commit 1c8063a into master Nov 20, 2020
@cidermitaina cidermitaina deleted the shin-color branch November 20, 2020 02:20
wmoai added a commit that referenced this pull request Nov 20, 2020
wmoai added a commit that referenced this pull request Nov 20, 2020
wmoai added a commit that referenced this pull request Nov 20, 2020
wmoai added a commit that referenced this pull request Nov 20, 2020
wmoai added a commit that referenced this pull request Nov 24, 2020
* feat: add Shin color (SHRUI-227) (#1141)

* test: update snapshot
wmoai added a commit that referenced this pull request Nov 24, 2020
* feat: add Shin color (SHRUI-227) (#1141)

* test: update snapshot

* test: fix lint
wmoai added a commit that referenced this pull request Nov 26, 2020
* feat: add Shin color (SHRUI-227) (#1141)

* test: update snapshot
wmoai added a commit that referenced this pull request Nov 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants