Skip to content

MudAlert: Fix content alignment issue #8734#8735

Merged
henon merged 1 commit intoMudBlazor:devfrom
neozhu:dev
May 2, 2024
Merged

MudAlert: Fix content alignment issue #8734#8735
henon merged 1 commit intoMudBlazor:devfrom
neozhu:dev

Conversation

@neozhu
Copy link
Contributor

@neozhu neozhu commented Apr 18, 2024

before
not alignment
"mud-alert-position justify-sm-start"

image
fixed
mud-alert-position align-center justify-sm-start

image

Copy link
Member

@danielchalmers danielchalmers left a comment

Choose a reason for hiding this comment

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

Good idea

.mud-alert-position {
flex: 1;
display: flex;
align-items:center;
Copy link
Member

Choose a reason for hiding this comment

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

nit: missing space between align-items: and center;

@henon henon changed the title fixed mudalert content alignment issue #8734 MudAlert: Fix content alignment issue #8734 Apr 18, 2024
@henon henon merged commit 6564c53 into MudBlazor:dev May 2, 2024
@henon
Copy link
Contributor

henon commented May 2, 2024

Thanks!

@ScarletKuro
Copy link
Member

As I understand this fixed this issue #8734 but it wasn't mentioned in the PR

@ScarletKuro ScarletKuro added bug Unexpected behavior or functionality not working as intended and removed PR: needs review labels May 2, 2024
@gabephudson
Copy link
Contributor

gabephudson commented Jul 22, 2024

@danielchalmers , this change has caused a visual difference from v6. Previously, the severity icon was aligned to the top (start) of the alert content. It is now aligned in the center of the mud-alert container. This is only noticeable with an alert that wraps to more than one line...
https://try.mudblazor.com/snippet/cYQeOhmQhDrzAUdQ

This was caused by the addition of "align-items: center;" to the .mud-alert-position css class.

While it's probably a subjective preference, I much prefer the icon to align with the top of the text for long alerts as it did in v6.

A workaround is to add .mud-alert-position { align-items: start; } to the applications custom css.

@danielchalmers
Copy link
Member

@gabephudson Could you post a before and after?

@gabephudson
Copy link
Contributor

Sure, here is a before (v6) image...
image

After
image

@danielchalmers
Copy link
Member

@gabephudson Thanks! I agree with you that it should be at the top. Do you want to submit a PR? CSS should be in here:

.mud-alert-position {
flex: 1;
display: flex;
align-items:center;
}

@gabephudson
Copy link
Contributor

I'm an ex-programmer and a highly technical product person now for our org, so I've never summitted a PR or have much practice with Git/GitHub. That said, I would love to learn. So I'm going to give it a try. :)

Let me know if there is anything out the box I may need to do, otherwise I'll have ChatGPT walk me through the process, lol. ;)

@henon
Copy link
Contributor

henon commented Jul 23, 2024

Here is a rough list of things to do:

  • Fork MudBlazor using your favourite git client
  • read CONTRIBUTING.md
  • compile the code from source and run the Docs.Server project to browse the MudBlazor docs
  • make your change and check with MudBlazor docs
  • no need to write a testcase for CSS changes only
  • push the change to your fork
  • go to mudblazor on github to create your PR, a button for that should show up under PRs
  • post before and after screenshots
  • We'll review and give feedback

@gabephudson
Copy link
Contributor

Thanks for the guidance, everyone! I have submitted the simplest of PRs. :) First time for everything. Hopefully I executed the PR correctly.

#9526

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Unexpected behavior or functionality not working as intended

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants