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

Update size of theme #243

Merged
merged 6 commits into from
Aug 19, 2019
Merged

Update size of theme #243

merged 6 commits into from
Aug 19, 2019

Conversation

im36-123
Copy link
Contributor

@im36-123 im36-123 commented Aug 7, 2019 •
edited
Loading

This PR include BREAKING CHANGE.

I updated font size of theme. Because the current theme file is different from the sketch file (that is defined in the SmartHR, Inc.)

@im36-123
chore: change some font size
5596d9f 
BREAKING CHANGE: remove size of tasting and trenta
@reg-suit
Copy link

reg-suit bot commented Aug 7, 2019 •
edited
Loading

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.

@auto-assign auto-assign bot requested a review from AtsushiM August 7, 2019 08:25
@im36-123 im36-123 changed the title Theme size Update size of theme Aug 7, 2019
@im36-123 im36-123 marked this pull request as ready for review August 7, 2019 10:33
ouji-miyahara
ouji-miyahara reviewed Aug 7, 2019
View reviewed changes
Copy link
Contributor

@ouji-miyahara ouji-miyahara left a comment

Choose a reason for hiding this comment

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

grande , short , tall といった呼称は用途や実体が類推しづらいので、これを機に変えたいです…。
また、デザインガイドラインではサイズや色などの定数をコンスタントケース（すべて大文字 & 単語の接続はアンダースコア）にしているのですが、合わせるのは難しいでしょうか？

@im36-123
Copy link
Contributor Author

im36-123 commented Aug 8, 2019

grande , short , tall といった呼称は用途や実体が類推しづらいので、これを機に変えたいです…。

自分も名前どうしようかなと思ってそのままにしたやつでした
フォントサイズの名前は S, M, L, XL とかですかね？

また、デザインガイドラインではサイズや色などの定数をコンスタントケース（すべて大文字 & 単語の接続はアンダースコア）にしているのですが、合わせるのは難しいでしょうか？

名前の変更だけなので問題ないです！ color も間違えている (パスカルケースになっている) のでそのあたりも直しちゃいます

@ouji-miyahara
Copy link
Contributor

フォントサイズの名前は S, M, L, XL とかですかね？

ここ、今日の定例で話しましょう！

@im36-123
Copy link
Contributor Author

im36-123 commented Aug 8, 2019

名前の変更だけなので問題ないです！ color も間違えている (パスカルケースになっている) のでそのあたりも直しちゃいます

この PR に乗せるとどこで変更があったかわかりにくそうだったので別 PR にしました
#245

@im36-123
Copy link
Contributor Author

im36-123 commented Aug 8, 2019

一旦 short, tall などの名称ですすめる。

@im36-123
Merge remote-tracking branch 'upstream/features/ttmz' into theme-size
4427243 
# Conflicts:
#	src/components/CheckboxLabel/CheckboxLabel.tsx
#	src/components/Field/Field.tsx
#	src/components/Input/Input.tsx
#	src/components/Pagination/PaginationItem.tsx
#	src/components/RadioLabel/RadioLabel.tsx
nabeliwo
nabeliwo approved these changes Aug 19, 2019
View reviewed changes
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.

gogo

@nabeliwo nabeliwo merged commit b578afa into features/ttmz Aug 19, 2019
@nabeliwo nabeliwo deleted the theme-size branch August 19, 2019 03:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ouji-miyahara ouji-miyahara ouji-miyahara left review comments

@nabeliwo nabeliwo nabeliwo approved these changes

@AtsushiM AtsushiM Awaiting requested review from AtsushiM

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

Successfully merging this pull request may close these issues.

None yet
3 participants
@im36-123 @ouji-miyahara @nabeliwo