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 support for modal with XL size and fix $type position #2842

Merged
merged 1 commit into from
Jun 14, 2024

Conversation

yuweiweiouo
Copy link
Contributor

@yuweiweiouo yuweiweiouo commented Jun 13, 2024

Feature: Added support for modal with XL (Extra large) size.

  • Updated modal component to handle XL size configurations.
  • Ensured consistent styling and behavior for the new size option.

Bug Fix: Corrected the misplaced $type variable.

  • Moved $type to the appropriate location in the layout template.
  • Verified that the fix does not impact other functionalities or cause regressions.

@@ -1,5 +1,5 @@
@push('modals-container')
<div class="modal fade center-scale"
<div class="modal fade center-scale {{$type}}"
Copy link
Member

Choose a reason for hiding this comment

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

The Bootstrap documentation states that size class selectors should be specified for the modal-dialog element. This can be seen at the following link: Optional Sizes.

Is there a reason why this was moved?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just wanted to clarify that the $type variable is meant for slide-right, not for modal-size.

I only found the combination .modal.fade.slide-right in the CSS, which suggests it's in the wrong spot. It actually works in the modal fade section.

So, I think it makes sense to move it there. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you can refer to here slide-right

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I definitely made a mistake in my reasoning.
Thank you for pointing that out. Your patch is already in the release.

@yuweiweiouo yuweiweiouo changed the title Add support for modal with MD size and fix $type position Add support for modal with XL size and fix $type position Jun 14, 2024
@tabuna tabuna merged commit 0b146b6 into orchidsoftware:master Jun 14, 2024
21 of 22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants