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

Error when using Max-height and max-width in global CSS class[Bug] #361

Closed
aterbo opened this issue Nov 12, 2021 · 8 comments
Closed

Error when using Max-height and max-width in global CSS class[Bug] #361

aterbo opened this issue Nov 12, 2021 · 8 comments
Labels
Docs Improvements or additions to documentation Resolved: Completed The issue has been resolved

Comments

@aterbo
Copy link
Contributor

aterbo commented Nov 12, 2021

Describe the bug
I set a global Class parameter to a custom CSS format and tried with max-width and max-height properties, but it gave me a weird, half-translucent modal. The modal is greyed out, and if I click on anything it closes the modal. I can't interact with the modal The header for the modal is also in the wrong spot, off to the left of the content

To Reproduce
Steps to reproduce the behavior:

  1. In Apps.razor set <CascadingBlazoredModal ContentScrollable="true" Class="custom-modal-class">
  2. In site.css add
.custom-modal-class {
    max-width: 90%;
    max-height: 90%;
}
  1. Open modal

Expected behavior
My goal is to have max height set to 95% and the scroll bars for anything taller than that. I'd ideally also like to have max width at 80% and then wrap content as necessary. Last resort is having horizontal scoll bars.

Screenshots
With max parameters set, showing bug. Note that the header, which says "IssuePopup Blazored" is off to the left of the body of the modal
With Max Size Parameters

Without parameters, showing modal normally
without max size parameters

Hosting Model (is this issue happening with a certain hosting model?):

  • Blazor Server

Additional context
To be honest, my CSS skills are pretty light. I'm willing to bet that the issue is me :)

@aterbo aterbo added Bug Something isn't working Triage Issue needs to be triaged labels Nov 12, 2021
@chrissainty
Copy link
Member

It looks like you're missing a z-index on your modal CSS.

.custom-modal-class {
    max-width: 90%;
    max-height: 90%;
    z-index: 1000;
}

Something like that.

@aterbo
Copy link
Contributor Author

aterbo commented Nov 13, 2021

Thanks for the response, Chris. That didn't entirely solve it, but it did move all of my editform input boxes within the modal up. I can now interact with the elements within the modal, but the modal background, box, etc, is still not rendering properly. I tried z-index of 1000 and 9999, and it renders the same.

Any other thoughts?

z-index 1000

@aterbo
Copy link
Contributor Author

aterbo commented Nov 13, 2021

In case it helps, I tried styling my modals with the css used in this discussion and it worked properly:

#297

The issue showed up when I tried using max height and width.

EDIT:
Sorry for chaining replies, but I also tried setting a custom background color with z-index. This is what I get now. The full modal box is visible now, but the header is still off to the side and the scroll bars are a bit wonky. It seems like trying to set a max width and height messes up the layout of the modal

z-index: 99999; background-color: aqua

z-index 99999 with background color

@larsk2009
Copy link
Collaborator

Can you share a solution with us that shows this problem? I'd be very interested to figure out what is going on!

@aterbo
Copy link
Contributor Author

aterbo commented Nov 15, 2021

Hi @larsk2009 I just made a repo showing the issue, with all the variations of CSS showing how it impacts the modal and what you can interact with:
BlazoredModalErrorTesting

If there's something I can do to help out with this please let me know. My time is fairly limited, but I'm happy to contribute to a PR if it solves this issue, even if I can only contribute documentation or something. I'm hoping to use Blazored in a larger project, but I need to get this resolved first. Thanks!

@aterbo
Copy link
Contributor Author

aterbo commented Nov 21, 2021

@larsk2009 I think I am on the right track for what's happening here. Please let me know if this sounds correct to you in general, or if I'm off base.

The main issue is that setting a custom class requires you override ALL of the properties specified in .blazored-modal yourself. In BlazoredModalInstance.razor.cs SetClass() method, your supplied Options.Class will be applied to the modal. If you do not supply a Class, then .blazored-modal is applied. There is no check that your supplied class has all of the properties necessary for proper modal display, i.e., all the properties in the default .blazored-modal

.blazored-modal {
    display: flex;
    z-index: 102;
    flex-direction: column;
    background-color: #fff;
    border-radius: 4px;
    border: 1px solid #fff;
    padding: 1.5rem;
    box-shadow: 0 2px 2px rgba(0,0,0,.25);
}

Since I was trying just to add height and width parameters, I was losing all of my other parameters defining the modal. That's why it wasn't rendering a background, border, shadow, etc. This is also why the header is moving off to the side when using max-height... Because it wasn't pulling in the flex and flex-direction parameters into the modal, but the sub divs have display: flex; things were getting all out of order.

If I supply all of the default values and add max-width, the box renders correctly and I can control the width.

However, I still can't limit the modal height. max-height doesn't seem to work properly, I think since it's set up as flex-columns. You can even see this in the sample project in CustomLayout.razor which tries to set max-height: 60vh; but the height actually isn't impacted. you actually can't see this in the sample as provided unless you add extra text to the modal message to increase the modal size. This is, fortunately, less critical to me than setting the max width.

I think step one of the solution would be to provide more documentation around the current custom styling requirements and what parameters are required.

A more robust solution would be to check the provided custom class on a parameter by parameter basis to override these default values individually and use the defaults where a user didn't provide their own. Anything without a default can be added to the class.

If you'd like, I can try to provide a pull request to programmatically check the supplied Class value to ensure that all of the default values are provided. I may need guidance on the best strategy to integrate this into the project, but my initial thought is just a helper method that would parse the provided string and compare it to the default values. It may be better to build it out similar to the ModalOptions class, where you can require each CSS parameter individually as a C# class parameter.

@larsk2009
Copy link
Collaborator

I don't think we should enforce anything when settings a custom class, as it is up to the implementing developer to decide how they want to style the modal. If we enforce it but a developer doesn't want to use that specific property, then what should they do? Or if we only overwrite and keep the basis, what if they want to remove something from that basis?

I do agree it might be a good idea to clarify this in the documentation if it is unclear.

I can't find the max-height thingy which you say is the examples, but that is probably me. Where exactly should I be looking?

@aterbo
Copy link
Contributor Author

aterbo commented Nov 24, 2021

Yeah, if there were some sort of programmatic enforcement, it would definitely need to allow the developer to overwrite or remove properties as they prefer, as long as they know what needs to be included. If you would prefer just to update the wiki, totally fine.

I think the main concern is to inform users that there are styles that are necessary to keep the modal from breaking, and these all needed to be provided in any custom class since the code will overwrite all defaults.

  • display: flex; (needed to keep default header alignment)
  • flex-direction: column;
  • z-index: 102; (needs to be higher than other classes for visibility)
  • background-color: #fff; (or any background)

If you want, I'm happy to write an update to the wiki clarifying this and provide it as a PR.

For the max height, I think I pointed you at the wrong thing. My fault. I'll take another look at it and get back to you.

@chrissainty chrissainty added Docs Improvements or additions to documentation Resolved: Completed The issue has been resolved and removed Bug Something isn't working Triage Issue needs to be triaged labels Dec 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Docs Improvements or additions to documentation Resolved: Completed The issue has been resolved
Projects
None yet
Development

No branches or pull requests

3 participants