allow render_react_component(name, props, class: "...") #9

gfx · 2016-12-08T02:35:41Z

We need to specify HTML class attributes to the wrapper div elements.

Can you review it please?

BTW I can't pass the test (#8) so the CI will fail :(

ljharb

Can you elaborate on why you need this? Specifically, why does any code besides hypernova's clientside JS need to reference this div?

ljharb · 2016-12-08T03:20:05Z

lib/hypernova/blank_renderer.rb

+    attributes = ''
+    options = job[:html_options]
+    if options && options[:class]
+      attributes << %{ class="#{options[:class]}"}


what if options[:class] includes a double quote? please include a test case that covers this.

How about cb90ed4 ?

gfx · 2016-12-08T04:58:02Z

Can you elaborate on why you need this? Specifically, why does any code besides hypernova's clientside JS need to reference this div?

This is because we need to apply CSS.

Out code includes CSS something like this:

foo {
  *[data-hypernova-key] {
     // blah blah blah
  }
}

This works but I think it is ugly and I don't want to depend on hypernova-specific things in CSS. If this PR is merged, I just add a CSS selector to the hypernova's div.

ljharb · 2016-12-08T06:31:58Z

@gfx what CSS are you trying to apply that can't be applied to the root element of your react tree?

gfx · 2016-12-09T05:58:49Z

Here you are:

  *[data-hypernova-key] {
    height: 100%;
  }

The height of a block element must be specified to the very element.

ljharb · 2016-12-09T06:54:31Z

To clarify, does this mean you're rendering the entire HTML content with react, and you want it to take up the whole screen?

juanca · 2022-07-11T13:01:42Z

I have a couple of use-cases:

Reduce unnecessary <div>s by styling the generated div instead of another wrapping <div>.
Apply a grid area name to the container to make the generated div fit an area of the page (sub-grid feature is not yet supported)
Apply a sticky position on either the generated div to make the react component sticky against the document.

Put it all together:

<body style="grid-template-areas: 'nav main'">
  <div style="grid-area: nav">
    <%= render_react_component('nav', { ... }, { style: "position: sticky; top: 0" }
  </div>
  <main>...</main>
  </div>
</body>

I could move the grid-area: nav to the generate div and make the react root component sticky. But either way, AFAIK, there is a necessity to style the generated div.

Unfortunately, I cannot render the whole application in React because our codebase is a mixture of accumulated tech stacks.

ljharb suggested changes Dec 8, 2016

View reviewed changes

gfx added 2 commits December 8, 2016 13:31

allow render_react_component(name, props, class: "...")

9515360

escape " in html attribute values

cb90ed4

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

allow render_react_component(name, props, class: "...") #9

allow render_react_component(name, props, class: "...") #9

gfx commented Dec 8, 2016 •

edited

Loading

ljharb left a comment

ljharb Dec 8, 2016

gfx Dec 8, 2016 •

edited

Loading

gfx commented Dec 8, 2016 •

edited

Loading

ljharb commented Dec 8, 2016

gfx commented Dec 9, 2016

ljharb commented Dec 9, 2016

juanca commented Jul 11, 2022 •

edited

Loading

allow render_react_component(name, props, class: "...") #9

Are you sure you want to change the base?

allow render_react_component(name, props, class: "...") #9

Conversation

gfx commented Dec 8, 2016 • edited Loading

ljharb left a comment

Choose a reason for hiding this comment

ljharb Dec 8, 2016

Choose a reason for hiding this comment

gfx Dec 8, 2016 • edited Loading

Choose a reason for hiding this comment

gfx commented Dec 8, 2016 • edited Loading

ljharb commented Dec 8, 2016

gfx commented Dec 9, 2016

ljharb commented Dec 9, 2016

juanca commented Jul 11, 2022 • edited Loading

gfx commented Dec 8, 2016 •

edited

Loading

gfx Dec 8, 2016 •

edited

Loading

gfx commented Dec 8, 2016 •

edited

Loading

juanca commented Jul 11, 2022 •

edited

Loading