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

Add padding to Alert softbuttons and adjust size #388

Merged
merged 3 commits into from
May 20, 2021

Conversation

jacobkeeler
Copy link
Contributor

@jacobkeeler jacobkeeler commented May 14, 2021

Fixes #369, #343

Testing Plan

  • I have verified that I have not introduced new warnings in this PR (or explain why below)
  • I have tested this PR against Core and verified behavior (if applicable, if not applicable, explain why below).

Core Tests

Test alerts with 1, 2, 3, or 4 softbuttons with various combinations of text and images

Core version / branch / commit hash / module tested against: 7.1.0
Proxy+Test App name / version / branch / commit hash / module tested against: RPC Builder JS

Summary

Adds padding to Alert softbutton images, also adjusts size of images when appropriate

Changelog

Bug Fixes
  • Fix style for Alert softbutton images

CLA

@jacobkeeler jacobkeeler changed the base branch from master to develop May 14, 2021 18:11
@jacobkeeler jacobkeeler changed the title Fix/alert softbuttons style Add padding to Alert softbuttons and adjust size May 14, 2021
@@ -102,6 +102,10 @@
padding: 15px;
}

.alert-buttons .soft-button-image-static {
padding: 5px;
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the padding should also be added for SubtleAlert Softbuttons (.v-subtleAlert-button-1 .soft-button-image and .v-subtleAlert-button-2 .soft-button-image)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed in 6f869b9

@@ -102,6 +102,10 @@
padding: 15px;
}

.alert-buttons .soft-button-image-static {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
.alert-buttons .soft-button-image-static {
.alert-buttons .soft-button-image, .soft-button-image-static {

Issue also applies to softbuttons with dynamic images

Screenshot from 2021-05-18 11-58-54

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed in 6f869b9

@@ -158,4 +162,9 @@
@include display(flex);
@include align-items(center);
@include justify-content(center);
}

.alert-button-4 .soft-button-image-static {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
.alert-button-4 .soft-button-image-static {
.alert-button-4 .soft-button-image, .soft-button-image-static {

Issue also applies to softbuttons with dynamic images

Screenshot from 2021-05-18 13-13-23

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addressed in 6f869b9

@jacobkeeler jacobkeeler merged commit f2b84b4 into develop May 20, 2021
@jacobkeeler jacobkeeler deleted the fix/alert_softbuttons_style branch May 20, 2021 21:10
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.

Alert soft button image alignment issues
2 participants