Skip to content

style – Adjust margin on elements inside the Alert component #2151

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

Merged
merged 2 commits into from
May 3, 2023

Conversation

B-T-D
Copy link
Contributor

@B-T-D B-T-D commented May 3, 2023

Description

The vertical alignment was inconsistent between different elements inside the Alert component. This PR adjusts the margins to make the Alert look better and properly aligned.

How the Alert component looked before:

before

After:

after

How Has This Been Tested?

Manually. Building locally with the change and visually inspecting.

Documentation

None

CheckList

  • PR title addresses the issue accurately and concisely
  • Updates Documentation and Docstrings
  • Adds tests
  • Adds instrumentation (logs, or UI events)

@boring-cyborg boring-cyborg bot added the area:frontend From the Frontend folder label May 3, 2023
@B-T-D B-T-D marked this pull request as ready for review May 3, 2023 16:59
$alert-warning-background: #fff0d4;
$alert-alert-background: #ffe4dd;
$alert-element-margin: auto 0 auto auto;
Copy link
Contributor

Choose a reason for hiding this comment

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

the top and bottom margins look good in the screenshot now, but the message is a little far over from the icon IMO. maybe we can have two separate margin values here, this one you have used for the alert-action, then auto auto auto 0 used for the alert-message so that we can have 0 on the left side like before

@B-T-D B-T-D merged commit 5cb36e1 into main May 3, 2023
@B-T-D B-T-D deleted the btd-table-alert-margin branch May 3, 2023 18:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:frontend From the Frontend folder
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants