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

DM / ホームチャンネルボタン内部で、アイコンだけの表示時に偏らないように #4370

Merged
merged 1 commit into from
Sep 18, 2024
Merged

DM / ホームチャンネルボタン内部で、アイコンだけの表示時に偏らないように #4370

merged 1 commit into from
Sep 18, 2024

Conversation

Pugma
Copy link
Contributor

@Pugma Pugma commented Sep 9, 2024 •
edited
Loading

margin -> gap で余白の取り方を変えることで、アイコンしか表示されないスマホモード時に表示が偏ることを解消しました

変更前
image

変更後
image

@Pugma Pugma requested a review from nokhnaton September 9, 2024 14:25
@Pugma Pugma marked this pull request as ready for review September 9, 2024 14:25
@github-actions GitHub Actions
Copy link

github-actions bot commented Sep 9, 2024

Preview (prod) → https://4370-prod.traq-preview.trapti.tech/
Preview (dev) → https://4370-dev.traq-preview.trapti.tech/

nokhnaton
nokhnaton reviewed Sep 10, 2024
View reviewed changes
Copy link
Contributor

@nokhnaton nokhnaton left a comment

Choose a reason for hiding this comment

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

中の要素が一個しかないからgapはつけなくても良さそう

@Pugma
Copy link
Contributor Author

Pugma commented Sep 10, 2024

PC 版だと「ホーム」とか「DM」って文字が表示されていて、それとの余白を保つための margin でした
それを gap に置き換えたという感じなので、gap は必要だと思います

TwoSquirrels
TwoSquirrels approved these changes Sep 10, 2024
View reviewed changes
Copy link
Member

@TwoSquirrels TwoSquirrels left a comment

Choose a reason for hiding this comment

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

gap は要素を均等に並べたいときに使うものですが、今回の場合は仮に要素が増えたとしても 4px 間隔で並べるべきなら適切だと思います。
また今回の場合、isMobile を使うと無駄にコードが複雑になってしまうと考えられるので、gap で適切だと思いました。

なので LGTM です。

nokhnaton
nokhnaton approved these changes Sep 18, 2024
View reviewed changes
Copy link
Contributor

@nokhnaton nokhnaton left a comment

Choose a reason for hiding this comment

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

なるほど。じゃあ良さそう

@Pugma Pugma merged commit 5c6a0ce into master Sep 18, 2024
10 checks passed
@Pugma Pugma deleted the fix/profileButtonMargin branch September 18, 2024 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nokhnaton nokhnaton nokhnaton approved these changes

@TwoSquirrels TwoSquirrels TwoSquirrels approved these changes

@kitsne241 kitsne241 Awaiting requested review from kitsne241

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

Successfully merging this pull request may close these issues.
3 participants
@Pugma @TwoSquirrels @nokhnaton