Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposal for the type attribute: omit it altogether #108

Closed
Zaid-Ajaj opened this issue Nov 9, 2019 · 6 comments
Closed

Proposal for the type attribute: omit it altogether #108

Zaid-Ajaj opened this issue Nov 9, 2019 · 6 comments

Comments

@Zaid-Ajaj
Copy link
Owner

Right now we have used the enum withType to specify the type of the Html input elements as follows:

Html.input [
  prop.className "form-control"
  prop.withType.password
  prop.valueOrDefault state.Whatever
]

The more I write it, the more weird it feels because withType is usually used the context of functions when chaining them using the |> operator but here they are just enums.

In the recharts binding, I took the liberty to experiment with the type attribute because some components like Line and Area also have this property and I have chosen to omit it and use the value directly as a valid attribute for that chart in subject:

Recharts.area [
    area.monotone
    area.dataKey (fun point -> nameof point.uv)
    area.stroke "#8884d8"
    area.fillOpacity 1
    area.fill "url(#colorUv)"
]

Notice here the attribute area.monotone is actually short-hand for type="monotone" from the JS equivalent and I am happy with in, this is nice too for the native properties like that:

Html.input [
  prop.password
  prop.className "form-control"
  prop.valueOrDefault state.Whatever
]

Very short, you get used it quickly and doesn't feel weird. I would personally also like props.types.password but again that feels like too many key strokes...

I was wondering what you guys think of it: @MangelMaxime, @cmeeren, @zanaptak, @Shmew

@Shmew
Copy link
Collaborator

Shmew commented Nov 9, 2019

I definitely agree that withType feels awkward. I feel like not having type' or withType before the attribute name could be confusing to people though, it gives way to ambiguity. If it wasn't making bindings for an existing API I'd absolutely favor just doing prop.password. At the same time, intellisense solves a lot of these issues raised.

In summary, I have no idea what's best. When I feel that way I usually just try to follow as closely to what the user would expect as possible, in this case it would probably be type'.

@MangelMaxime
Copy link
Contributor

The problem from my point of view is that without prefix this makes type a special case and also "pollute" the "global" scope. Not having a prefix is also making it a bit harder for people to discover the different type possible while having the prefix user can just do prop.<prefix>. and see the list.

I have seen several people using typ abbreviation for type perhaps we could use it too. And to make it more discoverable we could add ``type`` with a doc comment mentionning typ abbreviate for people not liking double backtick.

@Zaid-Ajaj
Copy link
Owner Author

Alright, stupid idea then hahah, thanks guys for the feedback 😄

@cmeeren
Copy link
Contributor

cmeeren commented Nov 9, 2019

The case seems closed, but for the record:

  • I agree with @Zaid-Ajaj that withType is weird
  • I agree with @Shmew and @MangelMaxime that we should keep some kind of type prefix (better discovery, avoids namespace pollution and potential collision, etc.).
  • I also agree with @Shmew that I'd rather see type'. It's searchable/discoverable, there's no ambiguity or confusion regarding whether the current with prefix means anything special since the apostrophe is not part of the word per se (i.e., it is an expected name), it's not uncommon to suffix names (especially reserved names) with apostrophe (or, as I mentioned in prop.inputType -> prop.input.type for better discoverability? #13 (comment), with other stuff, e.g. Elm calls it type_), and, to turn @Zaid-Ajaj's argument back around, you get used to it very quickly. I simply do not buy the argument from prop.inputType -> prop.input.type for better discoverability? #13 (comment) that an apostrophe suffix "always looks foreign and weird for F# beginners". To me it's not a valid argument; idiomatic F# looks weird to F# beginners. (Anything looks weird almost per definition when you're a beginner.) An apostrophe suffix is the least of an F# beginner's worries.

@zanaptak
Copy link
Contributor

zanaptak commented Nov 9, 2019

I would favor a better name while also keeping type in the name for intellisense/domain-representation purposes.

  • htmlType (a la htmlFor)
  • typeName (a la className)
  • typeValue
  • typeAttribute
  • typeText, typePassword, etc.
  • textType, passwordType, etc.
  • ``type`` (Chicken and egg -- we say backticks are too unfamiliar, but maybe if we all used them more...?)

@cmeeren
Copy link
Contributor

cmeeren commented Nov 9, 2019

  • ``type`` (Chicken and egg -- we say backticks are too unfamiliar, but maybe if we all used them more...?)

IMHO the main problem with double backticks isn't related to familiarity; it's that they're fairly "noisy" (hence my suggestion for type' which has 1 extra character, not 4).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants