Feat: MdDialog to target specific element with md-target attr #2254
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…-material into dialogs-target-el # Conflicts: # dist/components/MdDatepicker/index.css # dist/components/MdDatepicker/index.js # dist/components/MdDialog/index.css # dist/components/MdDialog/index.js # dist/components/MdDrawer/index.js # dist/components/index.css # dist/components/index.js # dist/vue-material.css # dist/vue-material.js # dist/vue-material.min.css # dist/vue-material.min.js
mikimoresi
changed the title
added 3 commits
August 18, 2020 11:58
mikimoresi
commented
Aug 18, 2020
Comment on lines
+117
to
+123
| { | ||
| name: "md-target", | ||
| type: "HTMLElement", | ||
| description: | ||
| "The dialog will be attached inside of the specificied HTML element instead of the body tag. The HTMLElement needs to have position relative CSS property.", | ||
| defaults: "null", | ||
| }, |
mikimoresi
commented
Aug 18, 2020
| @@ -1,13 +1,24 @@ | |||
| <template> | |||
| <md-portal> | |||
| <md-portal :mdTarget="mdTarget"> | |||
mikimoresi
commented
Aug 18, 2020
Comment on lines
+16
to
+17
| :mdTarget="mdTarget" | ||
| :md-fixed=" mdTarget ? false : true " |
There was a problem hiding this comment.
added mdTarget prop and md-fixed if mdTarget
mikimoresi
commented
Aug 18, 2020
| <transition name="md-dialog"> | ||
| <div class="md-dialog" v-if="mdActive"> | ||
| <div class="md-dialog" :class=" mdTarget ? 'with-target' : '' " v-if="mdActive"> |
There was a problem hiding this comment.
add with-target class in case of mdTarget
mikimoresi
commented
Aug 18, 2020
Comment on lines
+45
to
+48
| mdTarget: { | ||
| type: null, | ||
| default: null, | ||
| }, |
There was a problem hiding this comment.
added mdTarget prop
mikimoresi
commented
Aug 18, 2020
Comment on lines
+85
to
+91
| if (this.mdTarget) { | ||
| if (isActive) { | ||
| this.mdTarget.style.overflow = "hidden"; | ||
| } else { | ||
| this.mdTarget.style.overflow = "auto"; | ||
| } | ||
| } |
There was a problem hiding this comment.
forcing overflow behavioiur on target element if present to set the overlay fixed
mikimoresi
commented
Aug 18, 2020
Comment on lines
+145
to
+147
| &.with-target { | ||
| position: absolute; | ||
| } |
There was a problem hiding this comment.
added with-target css rule to bypass fixed position not able to target elements
mikimoresi
commented
Aug 18, 2020
| @@ -19,7 +19,9 @@ export default { | |||
| display: flex; | |||
| align-items: center; | |||
| justify-content: flex-end; | |||
| position: relative; | |||
| position: relative; | |||
| backface-visibility: hidden; | |||
There was a problem hiding this comment.
it fix hidden btn texts on chrome
mikimoresi
commented
Aug 18, 2020
| @@ -1,58 +1,62 @@ | |||
| <template> | |||
| <md-portal :md-attach-to-parent="mdAttachToParent"> | |||
| <md-portal :md-attach-to-parent="mdAttachToParent" :mdTarget="mdTarget"> | |||
There was a problem hiding this comment.
added mdTarget to mdPortal in mdOverlay
mikimoresi
commented
Aug 18, 2020
Comment on lines
+18
to
+21
| mdTarget: { | ||
| type: null, | ||
| default: null, | ||
| }, |
There was a problem hiding this comment.
added mdTarget prop
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Hello,
I have modified MdOverlay and MdDialog to add mdTarget prop.
In this way it is possible to make a dialog appear inside a specific element.
Passing in mdTarget prop an HTML element, the dialog and the overlay will appear in the given element and will not overlay the entire window.
Because of the position:fixed attribute that cannot target a specific element with position:relative, i have used position absolute both for MdDialog and MdOverlay in case of mdTarget exists. Plus, when mdTarget exists the overflow of the given element will be forced to "hidden" when dialog is active and go back in "auto" when dialog disappear.
The element in mdTarget MUST have position:relative
If you accept the proposal and maybe the pull request I will work on allowing the dialog to be scrollable if too long and maybe using a class on the mdTarget element instead of forcing the overflow style, this way anybody will be able to use that class for disable scrolling and doing any other desired thing like the required position:relative, the problem with this is that i would use pure js functions to add and remove classes while maybe there is some better approach, I will accept any suggestion.
Is my first pull request ever on an open source project (shame on me) so I don't know exactly how contributions works. So please be patient if I'm doing something wrong
EDIT: sorry for 10 comments below, i commented code lines to help reviews I didn't know it would create e reply for any comment