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

InlineAlert close button is removed from document flow to center alert text vertically #449

Merged
merged 1 commit into from
Aug 13, 2024

Conversation

ty2k
Copy link
Contributor

@ty2k ty2k commented Aug 13, 2024

Marking this as a draft so we can discuss whether we prefer to go this route or to make a smaller version of Button. See #444.

This PR updates the InlineAlert to remove the close button from the document flow, positioning it with position: absolute and a negative top value to center it.

The advantage of this approach is that we retain the 32px x 32px click target size for the close button, rather than rolling a smaller button. The disadvantage is that the code is more hacky and less declarative than using a smaller button and keeping the current InlineAlert code.

Single line of text, before and after:
Single line, before
Single line, after

Multiple lines of text, before and after:
Multi line, before
Multi line, after

@ty2k ty2k added this to the Components v0.2.0 milestone Aug 13, 2024
@ty2k ty2k requested a review from mkernohanbc August 13, 2024 21:37
@ty2k ty2k self-assigned this Aug 13, 2024
@mkernohanbc mkernohanbc linked an issue Aug 13, 2024 that may be closed by this pull request
@ty2k ty2k marked this pull request as ready for review August 13, 2024 22:16
@ty2k
Copy link
Contributor Author

ty2k commented Aug 13, 2024

Marking this as ready for review after our team discussion. Let's go forward with this plan for now.

@ty2k ty2k merged commit 576fc63 into main Aug 13, 2024
4 checks passed
@ty2k ty2k deleted the feature/inlinealert-close-button branch August 13, 2024 22:17
@ty2k ty2k mentioned this pull request Aug 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inline alert - fix close button positioning
2 participants