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

DataGrid Modal: Fixes fullscreen modal by applying min-height instead of height. #6020

Open
wants to merge 2 commits into
base: rel-1.7
Choose a base branch
from

Conversation

tesar-tech
Copy link
Collaborator

Description

Closes #5718

@tesar-tech tesar-tech requested a review from stsrki March 14, 2025 16:14
@@ -1,7 +1,7 @@
@typeparam TItem
@inherits BaseAfterRenderComponent

<Modal @ref="modalRef" Closing="@PopupClosing" Closed="@Cancel">
<Modal @ref="modalRef" Closing="@PopupClosing" Closed="@Cancel" Style="min-height:100%;height: auto;">
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would not call this a fix as it is not guaranteed that it would work for all providers. Also it feels "dirty" to me.

Instead, I would fix it by changing CSS where needed in our _modal.scss files. And see if it makes sense for all providers.

Copy link
Collaborator Author

@tesar-tech tesar-tech Mar 14, 2025

Choose a reason for hiding this comment

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

There is no problem with modals; the issue is only with _DataGridModal.razor, and the root cause lies in the document structure—the Form was causing the behavior.

Another issue with the DataGrid modal was the placement of the footer. It was positioned too low, requiring scrolling to reach it. In contrast, a normal modal has its footer fixed at the bottom of the screen.

With the Form now placed differently, both modals behave the same.

The git change seems crazy, but I really just change the placement of the form

Copy link
Collaborator

@stsrki stsrki Mar 15, 2025

Choose a reason for hiding this comment

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

There was a reason why Form was structured withing Modal. Because we need to be able to process ENTER key to submit modal form. Now that you have moved it inside of ModalMody, the buttons that are within ModalFooter no longer submits the form.

That's why CSS is a better way to fix this, as you will not break any behavior without knowing it.

You can also add a custom classname to the DataGrid modal so that you can specifically target it. eg b-datagrid-modal

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Editing the css is just a hot-fix (which you called dirty the first time), not a solution. The dataGrid modal will still behave differently then Modal (the "need to scroll to footer" issue).

We can fix both - the different modals behavior and the transparent issue - by structuring the form correctly and by
placing this

  <Button Type="ButtonType.Submit" PreventDefaultOnSubmit Disabled Display="Display.None"></Button>

inside the form. That without adding another piece of css.

Placing hidden button is technique used also in datagrid itself.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't have problem with CSS, but with inline and strongly typed styles.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ok.
So, what solution you prefer now?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Adding custom CSS classname to DataGrid and or DataGridModal, and then adding CSS as in your original commit seems like the best way.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I tried, but:
What do you mean by "adding custom css classname to DataGrid"? There is no datagrid.css. Or what am I missing?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hm, I was sure we had more of them. We have one in Blazorise.FluentUI2/styles/extensions/_datagrid.scss.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

To me, this is a statement about how Blazorise extensions are constructed—by using Blazorise components. For DataGrid, it’s almost entirely built without custom styles, so it would be a shame to break that for just one CSS class.

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.

2 participants