Shopify / turbograft

Hard fork of turbolinks, adding partial page replacement strategies, and utilities.
MIT License
214 stars 28 forks source link

A node should be able to have more than 1 refresh key #36

Open qq99 opened 10 years ago

qq99 commented 10 years ago

I propose that there's a good argument for having nodes with more than 1 key per refresh attribute.

Simulation:

<div id="modal" refresh="foo"></div>
<div id="container" refresh="foo">
  <div id="ohno">I contain client side state and must not be refreshed!</div>
  <div id="bar" refresh="bar">
    ...
  </div>
</div>

Say I want to refresh the innards of div#bar without refreshing div#container, because perhaps the controls that I'm using to refresh div#bar exist inside the div#ohno.

However, I also want to refresh the div#modal element, but I can't, because due to previous constraints, it's already got the key foo and refreshes along with div#container div. Now, I can't refresh just refresh=bar and div#modal without refreshing div#container

Allowing you to supply a list of refresh keys would be nice, so I could have my modal look like this:

<div id="modal" refresh="foo bar"></div>

and semantically this means that div#modal gets refreshed whenever foo-like or bar-like content changes.

I'm pretty sure this is not how things behave now

cc @patrickdonovan @nsimmons @pushrax @celsodantas

pushrax commented 10 years ago

I don't mind this, but it would be kinda difficult to do while still using querySelectorAll https://github.com/Shopify/turbograft/blob/76b4f6d2bda49d71c42aa8bdd2e4518a1ee8e1c1/lib/assets/javascripts/turbograft/turbolinks.coffee#L143

celsodantas commented 10 years ago

:+1: for this.

We could use this http://api.jquery.com/attribute-contains-selector/ $("[refresh*='#{key}']")

celsodantas commented 10 years ago

humm, maybe not. Would not work for compound words.

celsodantas commented 10 years ago

$("[refresh~='#{key}']") seems to work.

qq99 commented 10 years ago

yeah it's looking more & more like we may need to use jQ not sure if there's a nice DOM native way

nsimmons commented 10 years ago

In that case why couldn't we use a separate refresh key <div id="container" refresh="foo"> ?

Maybe I am missing something, but this does not seem necessary.

On Thu, Nov 13, 2014 at 11:22 AM, Anthony Cameron notifications@github.com wrote:

yeah it's looking more & more like we may need to use jQ not sure if there's a nice DOM native way

— Reply to this email directly or view it on GitHub https://github.com/Shopify/turbograft/issues/36#issuecomment-62919579.

qq99 commented 10 years ago

Well, previous constraints on the page make it so I cannot change the refresh key on the container (cause then the container does not update when another operation's refresh fires)