[Feature Request] Make dropdown position calculation compatible for embedded Vuetify apps

tarikhamilton commented 5 years ago

Problem to solve

If you have a Vuetify app that is embedded into a page where it is not taking up the full browser window, an app on a WordPress page for example, and you have a relative positioned element as its parent, dropdowns like VSelect will be offset.

Proposed solution

I tried digging into the source code, but could not really track down the logic, but the only way for me to get Vuetify's dropdowns to work in an embedded app situation, I had to remove all position: relatives from parent nodes. Add position: relative to v-app didn't work, so I'm guessing the calculation is based on the window, but somehow relative position parents alter the calculation.

I propose rewriting how the positioning calculation is performed and making sure v-app is the reference used for positioning.

jacekkarczmarczyk commented 5 years ago

Can you provide a reproduction?

tarikhamilton commented 5 years ago

@jacekkarczmarczyk

Here is a reproduction:

https://codesandbox.io/s/breaking-vuetify-dropdown-calculation-b6fw4

Tofandel commented 4 years ago

Same bug here, dropdown opens 20kilometers away from where it's supposed to in embedded app

https://github.com/vuetifyjs/vuetify/blob/master/packages/vuetify/src/mixins/menuable/index.ts

dimensions: {
      activator: {
        top: 0,
        left: 0,
        bottom: 0,
        right: 0,
        width: 0,
        height: 0,
        offsetTop: 0,
        scrollHeight: 0,
        offsetLeft: 0,
      },
      content: {
        top: 0,
        left: 0,
        bottom: 0,
        right: 0,
        width: 0,
        height: 0,
        offsetTop: 0,
        scrollHeight: 0,
      },
    },
//...
computedLeft () {
      const a = this.dimensions.activator
      const c = this.dimensions.content
      const activatorLeft = (this.attach !== false ? a.offsetLeft : a.left) || 0
      const minWidth = Math.max(a.width, c.width)
      let left = 0
      left += this.left ? activatorLeft - (minWidth - a.width) : activatorLeft
      if (this.offsetX) {
        const maxWidth = isNaN(Number(this.maxWidth))
          ? a.width
          : Math.min(a.width, Number(this.maxWidth))

        left += this.left ? -maxWidth : a.width
      }
      if (this.nudgeLeft) left -= parseInt(this.nudgeLeft)
      if (this.nudgeRight) left += parseInt(this.nudgeRight)

      return left
    },
    computedTop () {
      const a = this.dimensions.activator
      const c = this.dimensions.content
      let top = 0

      if (this.top) top += a.height - c.height
      if (this.attach !== false) top += a.offsetTop
      else top += a.top + this.pageYOffset
      if (this.offsetY) top += this.top ? -a.height : a.height
      if (this.nudgeTop) top -= parseInt(this.nudgeTop)
      if (this.nudgeBottom) top += parseInt(this.nudgeBottom)

      return top
    },

I think the bug is that the the closest relative element is missing in the calculation, so the position is calculated according to an assumed 0,0 instead of the app or whatever closest relative element which has a different offset

I'd say you'd need to add a .getBoundingClientRect() on the closest relative and subtract the offset from the calculation

if (!this.attach) {
  let p = this.getActivator();
  //Find the closest positioned element
  while (p && p.css('position') !== 'relative' && p.css('position') !== 'absolute') {
    p = p.parent();
  }
  if (p) {
    const rect = p.getBoundingClientRect();
    left -= rect.offsetLeft;
    top -= rect.offsetTop;
  }
}

So it would give this

computedRelativeOffset () {
      let p = this.getActivator();
      //Find the closest positioned element
     const isRelative =  (el) => {
        let pos = window.getComputedStyle(el, null).getPropertyValue("position");
        return pos === 'relative' || pos === 'absolute'
      }
      while (p && !isRelative(p)) {
        p = p.parent();
      }
      if (p) {
        const rect = p.getBoundingClientRect()
        return { left: -rect.offsetLeft, top: -rect.offsetTop }
      }
      return { left: 0, top: 0 }
    },
    computedLeft () {
      const a = this.dimensions.activator
      const c = this.dimensions.content
      const activatorLeft = (this.attach !== false ? a.offsetLeft : a.left) || 0
      const minWidth = Math.max(a.width, c.width)
      let left = 0
      left += this.left ? activatorLeft - (minWidth - a.width) : activatorLeft
      if (this.offsetX) {
        const maxWidth = isNaN(Number(this.maxWidth))
          ? a.width
          : Math.min(a.width, Number(this.maxWidth))

        left += this.left ? -maxWidth : a.width
      }
      if (this.nudgeLeft) left -= parseInt(this.nudgeLeft)
      if (this.nudgeRight) left += parseInt(this.nudgeRight)

      if (this.attach !== false) {
         left += this.computedRelativeOffset().left
      }

      return left
    },
    computedTop () {
      const a = this.dimensions.activator
      const c = this.dimensions.content
      let top = 0

      if (this.top) top += a.height - c.height
      if (this.attach !== false) top += a.offsetTop
      else top += a.top + this.pageYOffset
      if (this.offsetY) top += this.top ? -a.height : a.height
      if (this.nudgeTop) top -= parseInt(this.nudgeTop)
      if (this.nudgeBottom) top += parseInt(this.nudgeBottom)

      if (this.attach !== false) {
         top += this.computedRelativeOffset().top
      }

      return top
    },

I need to test it and do a PR, but I'm having trouble getting a cloned vuetify install to build in dev..

stephane303 commented 4 years ago

Is there a workaround meanwhile ? I am also embedding a Vuetify application, and I am confronted with the same problem.

dstearle commented 4 years ago

The Quick Run Down: I am building a calendar using vuetify to embed as a widget for wordpress sites and the popups for the scheduled events appear way below the widget due to OP.

My Current Solution: For now I have a band-aid solution for anyone else in my situation. Referring to the following link posted below, I used "attach" on the pop-up menu and set it to #app so that the pop-up menu will be attached to the div that the Vue app is generated in.

Ex:

` <v-menu v-model="selectedOpen" :close-on-content-click="false" :activator="selectedElement" attach="#app" offset-x

`

It's not a perfect solution but something for the mean time. Also if you want to you can set the CSS for the popup to be absolute then position the exact spot of the div with something like the following:

.centered { position: absolute ; top: 50%; left: 50%; }

Stack Overflow Reference: (https://stackoverflow.com/questions/50500842/how-to-attach-a-vuetify-v-menu-to-its-parent-element-in-a-scrollable-list)

hicreate commented 4 years ago

attach="#app" offset-x

@dstearle I'm experiencing a similar issue but all my components built on the v-menu component appear empty i.e. are empty dom elements. How did you setup your wordpress/vue/vuetify app?

I've created a wp plugin and enqueued the vue and vuetify files there which seems to work but then I get this strange behavious.

hicreate commented 4 years ago

Is there a workaround meanwhile ? I am also embedding a Vuetify application, and I am confronted with the same problem.

I too face this issue, is there a verified workaround as the solution from @dstearle doesn't seem to work.

hicreate commented 4 years ago

For me the solution I found that works is to use the attach prop without a value i.e.

<v-select :items="items" attach></v-select>

YmNIK13 commented 3 years ago

As a temporary solution.

Fixing the block with the root element example at codepen.io

<div id="fixed-main-block">
    <div id="app"></div>
</div>

<style>
    html {
        overflow: auto;
    }

    #fixed-main-block {
        position: fixed;
        top: 170px;
        max-height: calc(100vh - 224px);
        overflow-x: hidden;
        overflow-y: auto;
    }
    #fixed-main-block::-webkit-scrollbar {
        width: 8px;
        height: 8px;
    }

    .menubar-show #fixed-main-block {
        left: 268px;
        width: calc(100vw - 280px);
    }

    .menubar-hide  #fixed-main-block {
        left: 8px;
        width: calc(100vw - 16px);
    }

    #app > .v-application--wrap {
         min-height: auto;
    }
</style>

mschultz4 commented 3 years ago

For me the solution I found that works is to use the attach prop without a value i.e.

<v-select :items="items" attach></v-select>

This worked great for some cases, but not everything. For example:

It doesn't work inside a dialog. The position is correct but the dropdown is hidden.
Also, to my understanding, it cannot be applied to the data table, which has select dropdowns in its pagination.
If the parent (the element you are attaching to) is smaller than the dropdown, the dropdown will be cut off.
And, the most difficult to solve, scrolling the parent causes the space between the dropdown and its selector to widen by the length of the scroll.

enoh-barbu commented 3 years ago

that's why you (the framework in this case) should always render you menus/dialogs/etc into body root, positioned fixed, calculating coordinates by the target relative's coordinates....

WIStudent commented 3 years ago

I ran into the same issue today. Here is what I understood so far from reading through the source code: The menuable mixin is responsable for positioning the menu. It does it by calculating the top and left offset of the activator using getBoundingClientRect(). This is fine as long as the attach target has the same dimensions as the window menu1

But if the attach target is different from window, the offsets need to be calculated relative to the attach target menu2

Currently we do not have access to the attach target element in the menuable mixin. The actual target element seems to be calculated in the detachable mixin, but it is not saved inside the component. If we had access to the attach target element in the menuable mixin, we could simply calculate the difference between the clientBoundingRects of the activator and the attach target to get the correct offest. So we need to either save the attach target in the detachable mixin or recalculate the attach target inside the menuable mixin using the attach property.

There is one thing that I do not understand yet. According to this attach can be false, a valid query selector or a valid node. If it is false, the detachable mixin uses [data-app] as query selector. But if attach is '', true or 'attach' the detachable mixin does not attach the content to any target, impling that there are cases where no attach target exists. I do not know how these cases need to be handled in the menualble mixin.

Tofandel commented 3 years ago

I opened a PR which fixes the issue, some deeper tests are welcome mostly with elements with the attach prop, there is too many cases to check and no existing e2e tests (and in general there isn't a single e2e test in vuetify)

stephane303 commented 3 years ago

This issue also affect V-Tooltip, is it the same code ?

Tofandel commented 3 years ago

They all use the menuable mixin, but my testing only covers the vselect, it seems vTooltip overrides the position calculation so I might need to update it as well. I'll check if other components do the same and edit the playground with every one of those

Tofandel commented 3 years ago

The V-Tooltip seemed to be the only one with a calculation override, so it's also fixed

stephane303 commented 3 years ago

Is there an easy way to test it ? I tried to clone the repo and build the project, but I get some errors, are there instructions for building the project from the repo ?

On Fri, 21 May 2021 at 13:46, Adrien Foulon @.***> wrote:

The V-Tooltip seemed to be the only one with a calculation override, so it's also fixed [image: image] https://user-images.githubusercontent.com/6115458/119132331-dcc99500-ba3a-11eb-8dc9-d9bd042c11df.png

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/vuetifyjs/vuetify/issues/7703#issuecomment-845893408, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKYFKXLUFFZ7IRYD5HMMWLTOZBXTANCNFSM4H4M5P7Q .

Tofandel commented 3 years ago

https://vuetifyjs.com/en/getting-started/contributing/#setting-up-your-environment

Run yarn

Then yarn build

To get the playground run yarn dev (the entry file is packages/vuetify/dev/App.vue)

stephane303 commented 3 years ago

I just referring to build the project with your branch

On Fri, 21 May 2021 at 15:09, Adrien Foulon @.***> wrote:

https://vuetifyjs.com/en/getting-started/contributing/#setting-up-your-environment

Run yarn

Then yarn build

To get the playground run yarn dev (the entry file is packages/vuetify/dev/App.vue)

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/vuetifyjs/vuetify/issues/7703#issuecomment-845939010, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKYFKUHFC7AOMBFJFK4U2TTOZLSFANCNFSM4H4M5P7Q .

Tofandel commented 3 years ago

Yes it's the same process, it's just a fork, or do you mean that you want to include it as a npm package in another project? If the latter you should be able to do so by cloning my fork (git clone --single-branch --branch patch-2 git@github.com:Tofandel/vuetify.git && cd vuetify), building it by running yarn && yarn build

Then in your project run npm install path_to_cloned_vuetify/packages/vuetify

KaelWD commented 3 years ago

Then in your project run npm install path_to_cloned_vuetify/packages/vuetify

https://classic.yarnpkg.com/en/docs/cli/link/

vuetifyjs / vuetify

[Feature Request] Make dropdown position calculation compatible for embedded Vuetify apps #7703

Problem to solve

Proposed solution