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

<virtual data is= not working at all #2564

Closed
1 of 7 tasks
TheNavigateur opened this issue Mar 30, 2018 · 13 comments
Closed
1 of 7 tasks

<virtual data is= not working at all #2564

TheNavigateur opened this issue Mar 30, 2018 · 13 comments

Comments

@TheNavigateur
Copy link

@TheNavigateur TheNavigateur commented Mar 30, 2018

  1. Describe your issue:

<virtual data-is= not working at all inside a riot tag.

  1. Can you reproduce the issue?

http://plnkr.co/edit/yzIOVQ7rvzGUNdzhORjS?p=preview

...Change the <virtual to <my-tag2 to see that it should work.

  1. On which browser/OS does the issue appear?

Chrome Version 64.0.3282.186 (Official Build) (64-bit)

  1. Which version of Riot does it affect?

3.9.0

  1. How would you tag this issue?
  • Question
  • Bug
  • Discussion
  • Feature request
  • Tip
  • Enhancement
  • Performance
@GianlucaGuarini
Copy link
Member

@GianlucaGuarini GianlucaGuarini commented Mar 30, 2018

duplicate of #2511

Loading

@TheNavigateur
Copy link
Author

@TheNavigateur TheNavigateur commented Mar 31, 2018

But this was working in a previous version of Riot, so I'm not sure it's an exact duplicate. Riot should still be supporting the basic example provided, no? Otherwise it is a breaking change... right?

Loading

@GianlucaGuarini
Copy link
Member

@GianlucaGuarini GianlucaGuarini commented Mar 31, 2018

ah wait.. this feature is still supported even if it doesn't make any sense. I have checked your example with older riot versions and it works. I will look into it.
For now you can use data-is in the way it was meant to be used http://plnkr.co/edit/TZtwYCVh77bDFkS0KaNq?p=preview

Loading

@GianlucaGuarini
Copy link
Member

@GianlucaGuarini GianlucaGuarini commented Mar 31, 2018

fixed in riot 3.9.1 thank you

Loading

@TheNavigateur
Copy link
Author

@TheNavigateur TheNavigateur commented Mar 31, 2018

This feature makes perfect sense to me, and should continue to be supported. Obviously things like style on the tag declaration etc. would be ignored, but apart from that, it should just work as expected.

And there are real use cases for it, like a group of table rows among others in a table, etc.

Loading

@GianlucaGuarini
Copy link
Member

@GianlucaGuarini GianlucaGuarini commented Mar 31, 2018

@TheNavigateur I can't imagine any real use case for this feature but I would be interested in seeing an example where it makes sense for you

Loading

@TheNavigateur
Copy link
Author

@TheNavigateur TheNavigateur commented Mar 31, 2018

Well, my specific use case was to render a series of divs before other element(s), within a flex layout. That series of divs needs to be placed elsewhere in my application as well, but in a different container - hence why I needed to factor it out into a tag to avoid repetition of code.

This is not possible without <virtual data-is....

Please seriously consider supporting this properly and making it work as would be expected (if it isn't already)... I had always expected it to work (and it did) until it broke.

Loading

@GianlucaGuarini
Copy link
Member

@GianlucaGuarini GianlucaGuarini commented Apr 1, 2018

As I said before there is no useful case to support this feature. I am planning to remove it from the next major release. I have never seen a case where it makes sense unless you can clearly demonstrate what you are doing can't be done without it (with a concrete example)

Loading

@TheNavigateur
Copy link
Author

@TheNavigateur TheNavigateur commented Apr 1, 2018

I just described my use case (which exists in more than one place in my application). But here's a boiled down HTML of it:

<div style="display:flex;flex-direction:column">
    <virtual data is="bunch-of-divs"/>
    <button>OK</button>
</div>

Elsewhere in the application:

<div style="display:flex;flex-direction:row;padding:0px 8px;">
    <div>Options:</div>
    <virtual data-is="bunch-of-divs"/>
    <button>Add a new option</button>
</div>
<bunch-of-divs>
    <div>Complex div 1</div>
    <div>Complex div 2</div>
    <div>Complex div 3</div>
</bunch-of-divs>

I'm not sure about your response claiming "no useful case". The fact that I used it extensively before it broke surely demonstrates it has tangibly been very useful, in the most literal sense.

Loading

@GianlucaGuarini
Copy link
Member

@GianlucaGuarini GianlucaGuarini commented Apr 2, 2018

@TheNavigateur ok so you are basically styling your component contents outside of the component itself. I don't like this approach that goes against the idea of having self contained components that should not be modified from the outside environment. Thanks for your example.

Loading

@sourcegr
Copy link
Contributor

@sourcegr sourcegr commented Apr 2, 2018

self contained components that should not be modified from the outside environment

That is almost never the case. CSS outside your components modify the components, unless you include a reset.css in every component!

@TheNavigateur, may I ask why don you use a div like so

<div data-is="bunch-of-divs"></div>

Or even

<bunch-of-divs></bunch-of-divs>

instead?

Loading

@TheNavigateur
Copy link
Author

@TheNavigateur TheNavigateur commented Apr 3, 2018

Hi @papas-source , simply because the contents would lose the arrangement specified by the intended parents, namely <div style="display:flex;flex-direction:column"> and <div style="display:flex;flex-direction:row;padding:0px 8px;"> in the given example.

@GianlucaGuarini Yes when contents share the same structure but different layout arrangements among other elements. Of course the extent to which a component template can be self-contained, I do that, but that's normally not 100% in my application (global font, for example).

Loading

@sourcegr
Copy link
Contributor

@sourcegr sourcegr commented Apr 3, 2018

@TheNavigateur you know you can accomplish what you ask by slightly modifing your CSS rules, right? I mean you only have problem with flex-direction: row that can be easily be solved like this.

<div style="display:flex; flex-direction:row;">
  <div>this is head</div>
  <my-tag2 style="display:flex; flex-direction:row;"></my-tag2>
  <button>Add a new option</button>
</div>

Besides than that, I believe that components should be contained in a parent element, it just feels more natural to me.

But I can see your use case and need. 👍

Loading

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants