-
-
Notifications
You must be signed in to change notification settings - Fork 4k
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
[modals]: style btn-close only when in .modal-header #1925
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@blikblum AFAIK this is expected behaviour. As not all modals and card have headers. What problem are you trying to solve? |
Put a tag component inside a modal: <div class="modal-body">
<span class="tag"> Test <button type="button" class="btn-close"></button></span>
</div> The close button of all tags will be displayed in top right corner. See image in OP |
Original bootstrap code only style close button inside header. To support close btn outside a header, IMO tabler should do using a dedicated markup |
In that case I would change it that it needs to be a direct descendant of modal-header or modal-content. That way both situations will work. |
WalkthroughThe changes involve a restructuring of the modal styles in the Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Modal
participant CloseButton
User->>Modal: Open modal
Modal-->>User: Display modal content
User->>CloseButton: Click close button
CloseButton->>Modal: Close modal
Modal-->>User: Hide modal content
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/scss/ui/_modals.scss (2 hunks)
Additional comments not posted (2)
src/scss/ui/_modals.scss (2)
Line range hint
1-10
: LGTM! Improved specificity for close button styling.The selector
> .btn-close
ensures that the close button is styled correctly when it is a direct child of.modal-container
or.modal-header
. This change aligns with the PR's objective to address styling issues.
Line range hint
2-10
: LGTM! Enhanced specificity for.btn-close
.The change to
> .btn-close
ensures that only direct child close buttons are styled, which prevents styling issues with nested buttons.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/scss/ui/_modals.scss (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- src/scss/ui/_modals.scss
I will check this tomorrow when all builds are finished |
@rjd22 many thanks |
Currently if you put a .btn-close element in Modal body, the element is displayed in top right.
See below how the below code (a .tag with a .btn-close) is rendered (look at the smaller close below the header close):
This PR fixes it
Summary by CodeRabbit