leptos-rs / book

The home for the Leptos book, which can be found deployed at https://book.leptos.dev
MIT License
80 stars 60 forks source link

Demonstrate Dynamic Styles #28

Closed gihrig closed 9 months ago

gihrig commented 9 months ago

Changes allow demonstration of the Dynamic Styles code. It's important (imo) that docs allow readers to put the covered concepts into practice in their own environments. This is the only way they can "know" that covered material works for them.

Two changes:

  1. Added buttons to exercise code showing dynamic styles. Increment vaules in steps of 10 to make motion more obvious.

  2. Remove "columns" style. The syntax provided doesn't work. The concept is not really applicable to the example (imo).

gbj commented 9 months ago

It's important (imo) that docs allow readers to put the covered concepts into practice in their own environments.

I agree with this in theory, but I'm not convinced that every single example needs to provide a complete, copy-and-pasteable app. The previous chapter just showed how to update a signal: this example is building on that. If you think it's really important, then it's probably best handled by turning this <div> into a <button> (as in the class example above it) so it remains a snippet of the same size.

Remove "columns" style. The syntax provided doesn't work. The concept is not really applicable to the example (imo).

Could you clarify this? The syntax provided absolutely does work, and is a way of updating a CSS variable, which is a style property like any of the others in the example.

gihrig commented 9 months ago

To your first point: copy/paste example aren't necessary.

I sort of agree, maybe copy/paste vs complexity is a delicate trade-off. I find working examples helpful, especially when I'm new to something. When I start with working code and fiddle from there, at least I know that if it doesn't work it's because I broke something.

Particularly since this page is building on an example, I expected that to continue. Demonstration code can still be minimal. I would just like to see a clear path to a working example.

From the Dynamic Attributes section:

Let’s add another element to our view:

<progress
      max="50"
     // signals are functions, so this <=> `move || count.get()`
      value=count
/>

This is perfect. It's clear what is to be done to implement the example code.


To your second point: I get that you are objecting to too much unnecessary code?

I had that feeling as I committed this, so here's a few ideas:

  1. As you suggest, merge the instructional part of the code into one button.
  2. There's still the issue of advancing two values, x and y. Is it necessary to move the text in two dimensions? Since x an y are identical code beyond x and y, I propose to remove y. The principle is still demonstrated.

Finally, The issue of columns and syntax:

is a way of updating a CSS variable

Sorry, I was trying so hard to grok Leptos that I didn't even think of CSS variables. I added a comment to clarify what was being done there.

gihrig commented 9 months ago

I have simplified the example considerably.

Thanks for your critique. I like it much better now. 😀

diversable commented 9 months ago

It's important (imo) that docs allow readers to put the covered concepts into practice in their own environments.

I agree with this in theory, but I'm not convinced that every single example needs to provide a complete, copy-and-pasteable app. ...

FYI - there is a feature of mdbook that allows you to hide lines of code in the output documentation, so theoretically we could provide copy-pasteable code for some examples without cluttering up the book/code too much.

To hide a line of code, start the line with a hashtag: # <hidden line of code> See here: https://rust-lang.github.io/mdBook/format/mdbook.html#hiding-code-lines

just so you're aware..

hiding lines of code might allow us to highlight the code that matters while still providing working code for newer Rustaceans. On the other hand, it's more work to do.

if you think it's a good idea, though, it's something I could start working towards bit by bit throughout the book...

gbj commented 9 months ago

Thanks, this looks really great! I added a comment to slightly clarify the distinction between setting the overall attribute style and toggling individual properties: style: literally toggles individual pieces of the CSSStyleDeclaration individually.

Sorry, I was trying so hard to grok Leptos that I didn't even think of CSS variables. I added a comment to clarify what was being done there.

Yeah I had also forgotten that columns is itself a CSS property so this could be confusing. It is indeed toggling a CSS variable -- thanks for the added comment there.