Giter Site home page Giter Site logo

styles and sublists about feliz HOT 7 CLOSED

zaid-ajaj avatar zaid-ajaj commented on July 19, 2024
styles and sublists

from feliz.

Comments (7)

Zaid-Ajaj avatar Zaid-Ajaj commented on July 19, 2024 1

Sorry, I have to say no. Longer term I'm happy to contribute but for now you are in rapid-beta-release mode and I'm not prepared to be on the hook for any short-term promises.

@zanaptak That's fine, the feature got implemented already :)

Fair enough. Though you can cheat with compatibility by having props implement the empty ReactElement interface. If you were to change the grouping, and use a nested list for props instead of for child elements, you could isolate the cheat to a single appropriately documented helper.

I know it is easily possible, but it will require not only cheating the type-system but also the helper that needs to distinguish the props from the elements apart and that means I would to include metadata of each element plus metadata of the prop keyword to be able to distinguish them in runtime: this is not a goal I want to pursue in a base library where it should the least possible amount of helper and magic because code from here would be used over and over and it would start to hurt performance and bundle sizes, so it's a no-go

I will things as they are for now

from feliz.

MangelMaxime avatar MangelMaxime commented on July 19, 2024

For styles I was preferring to have the valid options "attached", and not the somewhat verbose method of having separate option types where you repeat the css property. Also, unit alternatives attached instead of extra parenthesized args.

Seems interesting 🤔

Maybe controversial, but the children being in a nested sublist isn't really that valuable from a coding ergonomics standpoint, beyond React doing it that way. It's an extra wasted indent that will compound on every nesting level.

It does not only react doing HTML is also doing it that way, but you do also have a list of children for a given element.

By using a prop.content property this allows us to do only one iteration over the list in order to create an object and not two which increase potentially the performance, and also reduce the bundle size increase to zero on this specific point.

It can be a mixed list using interfaces or DUs, the Html helpers can partition by attributes vs. elements and build the necessary React API objects without imposing that structure on the dev.

This helps structure the code so people don't mix children and properties like that:

prop.Id ""

Html.button [ ]

prop.style [ ]

Html.h1 [ ]

// etc

Let's keep things separate please 😄

from feliz.

Zaid-Ajaj avatar Zaid-Ajaj commented on July 19, 2024

Thanks for this, I'm trying it out immediately!

As it happens I was also working on a new DSL almost just like this. Wasn't a fan of the traditional double-list parameter style even after many months of giving it a chance. Rather than compete I'll choose to be happy you saved me a bunch of work. 😃

Thanks for trying this out, we are still testing the waters with how it will look like in a large app, keep us posted please 😄

For styles I was preferring to have the valid options "attached", and not the somewhat verbose method of having separate option types where you repeat the css property. Also unit alternatives attached instead of extra parenthesized args.

I love this! Of couse it will not work for all properties because some of them like margin and border would want different units for different parameters so we can't replace the functions altogether but I would like to add the attached properties for distinct properties like display and textAlign etc. this would simplify a lot getting rid of specific types such as IDisplay and ITextAlignment etc. but also conditional styles are easier:

style.display (if true then display.none else display.block)

// vs

if true then style.display.none else style.display.block

This is the implementation I would like to follow:

[<Erase; RequireQualifiedAccess>]
module style =
    [<Erase; RequireQualifiedAccess>]
    type display =
        static member inline flex : IStyleAttribute = Interop.mkStyle "display" "flex"
        // etc.

but also keep things like style.margin 5 valid next to style.margin.px 5 and style.margin.vh 100

@zanaptak Can you please help out with these changes 🙏? the types and docs are already there but they need to be moved around.

As for the second proposition

Maybe controversial, but the children being in a nested sublist isn't really that valuable from a coding ergonomics standpoint, beyond React doing it that way. It's an extra wasted indent that will compound on every nesting level.

It can be a mixed list using interfaces or DUs, the Html helpers can partition by attributes vs. elements and build the necessary React API objects without imposing that structure on the dev. (This also eliminates the need for the prop.content helper since you can just use Html.text directly.)

Although I personally liked this in the beginning before implementing this library, I don't think it is a good idea now because it breaks compatibility: props and react elements would have to have the same type in order to put them in the list, this means that props would be some kind of elements or elements would have to some kind of props and they won't compose anymore with third-party libraries. Also this follows exactly the structure of React so I would like to keep it as is right now

from feliz.

Zaid-Ajaj avatar Zaid-Ajaj commented on July 19, 2024

Implemented attached styles 😄 and will be available in v0.16! see README

from feliz.

zanaptak avatar zanaptak commented on July 19, 2024

It does not only react doing HTML is also doing it that way, but you do also have a list of children for a given element.

Not sure if I was clear enough, I'm referring specifically to putting child html elements under prop.children [] instead of directly under the parent Html.div [] element. This is arising directly from the chosen list representation of React props and is not like writing HTML (or even JSX I don't think, but not too familiar), e.g. we don't put child elements in a nested <children></children> or children="…" container in HTML, we put them right under the parent element.

By using a prop.content property this allows us to do only one iteration over the list in order to create an object and not two which increase potentially the performance, and also reduce the bundle size increase to zero on this specific point.

I'm guessing if necessary it should be possible to write a native JS function that could do it in one pass using mutable objects, I think it just needs to build a POJO and child array in-place unless there's more involved I'm overlooking. I don't really know much about bundle sizing so if that's an issue I don't have any ideas for that.

This helps structure the code so people don't mix children and properties like that:

This is a variation on problems that already exist by choosing a list DSL over a stricter abstraction, and is unreasonable to be singled out for special objection while the others are tolerated.

We can already split attributes in the new DSL.

Html.div [
    prop.id "id"
    prop.className "asdf"

    prop.children [

        Html.div [
            prop.content "child1"
        ]

        Html.div [
            prop.content "child2"
        ]

    ]

    prop.name "name"
    prop.key "key"
]

We can also duplicate and overwrite items -- which prop.children block wins?

Html.div [

    // Which children block will render?

    prop.children [
        Html.text "THIS ONE?"
    ]

    prop.children [
        Html.text "OR THIS ONE?"
    ]

    prop.id "id" ; prop.className "class"
    prop.name "name" ; prop.content "" ; prop.key "key"
    prop.style [
        style.display.flex
        style.backgroundColor.aquaMarine
    ]
]

(The answer is that the empty prop.content at the bottom clobbers them both.)

These are things we put up with in the name of convenience, that users already have to learn to avoid on their own without compiler help, the proposed alternative would fall into the same category.

@zanaptak Can you please help out with these changes 🙏? the types and docs are already there but they need to be moved around.

Sorry, I have to say no. Longer term I'm happy to contribute but for now you are in rapid-beta-release mode and I'm not prepared to be on the hook for any short-term promises.

Although I personally liked this in the beginning before implementing this library, I don't think it is a good idea now because it breaks compatibility: props and react elements would have to have the same type in order to put them in the list, this means that props would be some kind of elements or elements would have to some kind of props and they won't compose anymore with third-party libraries. Also this follows exactly the structure of React so I would like to keep it as is right now

Fair enough. Though you can cheat with compatibility by having props implement the empty ReactElement interface. If you were to change the grouping, and use a nested list for props instead of for child elements, you could isolate the cheat to a single appropriately documented helper.

Html.div [  // ReactElement seq -> ReactElement

    // Cheat: implements ReactElement, will "keyValueList" into props object internally
    props [
        prop.id "a"
        prop.style [
        // ...
        ]
    ]

    // Everything else become regular children
    // Any ReactElement (new DSL, old DSL, component, etc.)

    Html.div [
        // ...

    ]

    Html.div [
        // ...

    ]
]

from feliz.

MangelMaxime avatar MangelMaxime commented on July 19, 2024

Not sure if I was clear enough

You were pretty clear about the prop.children thing :)

We can also duplicate and overwrite items -- which prop.children block wins?

We can ask the same question for any property :) It will always be the last one who wins.

I still don't think mixing props and ReactElement is a good thing. However, your last proposition is interesting at first glance.

I don't see big cons about it because you still have the (visual) distinction between the props and children.

So depending on @Zaid-Ajaj opinion perhaps we can try to compare a big block of codes with the different styles to make a choice. It's already planned to do it between several other code format so one more or one less :)

from feliz.

zanaptak avatar zanaptak commented on July 19, 2024

After rereading this and looking at it a bit more I'm also thinking grouped props are better than the original mixed proposal.

from feliz.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.