Native review for coreytrampe/elm-vendor about package.elm-lang.org HOT 12 CLOSED

Exactly what i need for a major update on flex-html. As it's such a small module, it should hopefully not take too long to review & publish it :) Looks good to me, great job 👍

from package.elm-lang.org.

This line should be this:

return elm.Native.Vendor.values = { prefix: vendorPrefix };

This makes it so the whole module does not get evaluated tons of times, that's why the prelude checking for .values is needed.

from package.elm-lang.org.

Also, to set expectations, this will not be the recommended way of doing things in the future. For the sake of making the CSS stuff work, I think it is worth it to make an exception here, but in the next months I think we well need to redo a bunch of the native modules out there. Are you okay with doing this work at some point in the future?

from package.elm-lang.org.

@evancz, are there plans to make it possible to have external CSS code published along modules? Or how would the recommended way solve modules that require heavily on CSS features?

I'm ok to help out @coreytrampe for stuff like this btw, if he needs a helping maintaining hand :)

from package.elm-lang.org.

Right now folks just distribute CSS alongside the generated Elm. Put a class in the Elm code. I don't have anything fancier than that planned, and I have a feeling the community can come up with better things than me on this question.

from package.elm-lang.org.

Ok, fixed that. Thanks, Evan.

I'll be happy to update when the time comes.

from package.elm-lang.org.

Did you edit it based on an email notification? I edited the code snippet, I got it wrong at first. It needs to have the .values part.

I looked at docs a bit. Does it make sense to return a string? If so, what are all the possible strings? How should I find it out? Should they all be listed in the docs? Would it make sense to represent it as a union type?

type Vendor = Moz | Webkit | MS | Opera

That way there is no unexpected case for anyone. You can decide for yourself what is the right tradeoff, but I just want to make sure to ask :)

from package.elm-lang.org.

Lol, yeah. Glad you noticed.

That seems like a good idea. The usage example becomes:

import Vendor

displayValue : String
displayValue =
    if Vendor.prefix == Vendor.Webkit
    then "-webkit-flex"
    else "flex"

Same code, but with a much stronger contract. Dig it.

from package.elm-lang.org.

LMK when it's updated

from package.elm-lang.org.

Alright, whenever you have the chance, buddy.
Sorry about the churn!

from package.elm-lang.org.

I quite like this, thanks for the work @coreytrampe 👍

from package.elm-lang.org.

Okay, should be publishable! Thanks for going through the review process!

from package.elm-lang.org.