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

Fix modal positioning in panel headers #1910

Merged
merged 1 commit into from
Apr 20, 2022

Conversation

jovyntls
Copy link
Contributor

@jovyntls jovyntls commented Apr 18, 2022

What is the purpose of this pull request?

  • Documentation update
  • Bug fix
  • Feature addition or enhancement
  • Code maintenance
  • Others, please explain:

Fixes the positioning issues from #1909

Overview of changes:
Fixes the styles from panel headers which was disrupting modal positioning.

  • Change margin: 0 to margin-bottom: 0 which seems to be the minimal change that retains panel header styling without affecting modal positioning

Anything you'd like to highlight / discuss:
The CSS here was inherited from MarkBind/vue-strap, where it was first implemented for minimal panels

Testing instructions:
Modals in panel headers should now be positioned correctly.

Proposed commit message: (wrap lines at 72 characters)

Fix modal positioning in panel headers

Checklist: ☑️

  • Updated the documentation for feature additions and enhancements
  • Added tests for bug fixes or features
  • Linked all related issues
  • No unrelated changes

@jovyntls jovyntls marked this pull request as ready for review April 18, 2022 03:47
@jovyntls jovyntls requested a review from tlylt April 18, 2022 03:47
@karnatisrinivas
Copy link

https://deploy-preview-1910--markbind-master.netlify.app/userguide/syntaxcheatsheet

I assume the above is the deployment preview of commit @jovyntls made, That's some great work. But when I navigated to Page Navigation menus in syntax cheat sheet. I still get two modals that overlap with each other.
Kindly review once.

1 similar comment
@karnatisrinivas
Copy link

https://deploy-preview-1910--markbind-master.netlify.app/userguide/syntaxcheatsheet

I assume the above is the deployment preview of commit @jovyntls made, That's some great work. But when I navigated to Page Navigation menus in syntax cheat sheet. I still get two modals that overlap with each other.
Kindly review once.

@jovyntls
Copy link
Contributor Author

Hi @karnatisrinivas, thanks for looking through! However, this PR is not meant to fix the two overlapping modals. It's only meant to fix modals being positioned wrongly when they are used in a panel header, because the root cause of the overlapping modals is different.

@karnatisrinivas
Copy link

Oh my bad , i wasn't aware of that, I thought it meant to fix the issue. LGTM

Copy link
Contributor

@tlylt tlylt left a comment

Choose a reason for hiding this comment

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

LGTM

@tlylt tlylt requested a review from ong6 April 19, 2022 10:42
Copy link
Contributor

@ong6 ong6 left a comment

Choose a reason for hiding this comment

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

LGTM

@ong6 ong6 merged commit 104e811 into MarkBind:master Apr 20, 2022
@jonahtanjz jonahtanjz added this to the 4.0 milestone Apr 20, 2022
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.

5 participants