vue-select icon indicating copy to clipboard operation
vue-select copied to clipboard

add appendTo option and mark appendToBody option as deprecated

Open rx-837 opened this issue 4 years ago • 4 comments

rx-837 avatar Sep 01 '21 22:09 rx-837

Thanks - I like this proposal.

I was hoping to change the prop type from String to HTMLElement, as it cuts out a bunch of boilerplate needed to determine the container, and is a stricter API. That part works great, but unfortunately, setting type: HTMLElement breaks server side rendering, as node doesn't have the HTMLElement constructor.

I'd like to find a way to support directly passing an element as the prop value, since it's likely to be a template ref. Let me know if you've got any ideas on that.

Also, I reverted the changes to package.json. Releases are handled automatically based on commit messages, so no need to update.

sagalbot avatar Oct 07 '21 16:10 sagalbot

@sagalbot

Your motives to use strict API are clear, as is the fact that there are no DOM entities in the nodejs environment.

What the benefits your expect when pass HTMLElement type inside props? As far as i know there is no way to call the constructor function explicitly of Element(HTMLElement).

If the benefits is only to validate prop type on "Vue" level, then you can define type as Object, and validate type manually inside "mounted" hook(this.appendTo instanceof HTMLElement). Also you can use own prop validatator - validator(value) { return value?.tagName !== '' }

Can you accept my pull request as is? There are no breaking changes, but issue with custom container has been resolved.

If you find way to pass ref to HTMLElement inside props you can change prop declaration to { type: [String, HTMLElement] }.

"String" usage is more flexible then "HTMLElement" e.g. i have a portal inside root layout, passing ref to components depth is hell, simpler use a selector.

rx-837 avatar Oct 19 '21 07:10 rx-837

@sagalbot what about pull request? As can i see still it not merged(

rx-837 avatar Nov 21 '21 20:11 rx-837

@rx-837 @sagalbot I think this change to append-to is really useful. Would this be merged soon?

daniel-anh-nguyen avatar Jan 13 '22 13:01 daniel-anh-nguyen