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

inheritFromParent -> inherit' #12

Closed
cmeeren opened this issue Aug 11, 2019 · 13 comments
Closed

inheritFromParent -> inherit' #12

cmeeren opened this issue Aug 11, 2019 · 13 comments

Comments

@cmeeren
Copy link
Contributor

cmeeren commented Aug 11, 2019

Since inherit is a reserved keyword, what about simply adding an apostrophe (inherit') instead of coming up with another name (inheritFromParent)?

@Zaid-Ajaj
Copy link
Owner

Same as my comment here I am trying to avoid apostrophe in naming. inheritFromParent derives the naming from what it does and considering how uncommon this construct will be used (ofc. just a guess, I don't have numbers) it is good enough

@cmeeren
Copy link
Contributor Author

cmeeren commented Aug 12, 2019

Replying to the above as well as your comment in that issue: I'm not an F# beginner, but when I was, I didn't see any problem with ' (neither do I now). In fact, I thought it was a great way to avoid clashes with reserved keywords and used it wherever needed (and still do). That's me, though. :)

@cmeeren
Copy link
Contributor Author

cmeeren commented Aug 12, 2019

(For completeness: Another alternative is to use double backticks, ``inherit``, but I vastly prefer a single apostrophe to double backticks in this case.)

@cmeeren
Copy link
Contributor Author

cmeeren commented Aug 24, 2019

Just want to note that I consistently append ' to reserved keywords in Feliz.MaterialUI. That way:

  • Users intuitively understand that e.g. color.default' refers to the MUI default color
    • I expect users would be confused by alternative names like color.standard or color.normal and annoyed at more verbose alternatives like color.defaultColor, and all of these could also clash with new additions in MUI
  • I avoid having to come up with custom names for a lot of props

IMHO it looks great in usage, too. :) I suggest doing the same in Feliz, but it's ultimately your call, of course. (And I'm always open to hearing better ways of doing things in Feliz.MaterialUI)

@Zaid-Ajaj
Copy link
Owner

I agree it makes sense in Material UI context but not really in this base library because of how uncommon it is used and it is here the only one (I think along with type' for which I used inputType)

I will leave this issue open for a while, maybe someone can enlighten our subjectiveness :)

@cmeeren
Copy link
Contributor Author

cmeeren commented Aug 31, 2019

Forgot to mention in #42 but I'd just like to be open about the fact that I added one such apostrophe name (ariaAutocomplete.inline'). If you don't like it, feel free to rename it. :)

Feliz/src/Properties.fs

Lines 871 to 885 in 0615e78

/// Indicates whether user input completion suggestions are provided.
///
/// https://www.w3.org/WAI/PF/aria-1.1/states_and_properties#aria-autocomplete
[<Erase>]
type ariaAutocomplete =
/// A list of choices appears and the currently selected suggestion also
/// appears inline.
static member inline both = Interop.mkAttr "aria-autocomplete" "both"
/// The system provides text after the caret as a suggestion for how to
/// complete the field.
static member inline inline' = Interop.mkAttr "aria-autocomplete" "inline"
/// A list of choices appears from which the user can choose.
static member inline list = Interop.mkAttr "aria-autocomplete" "list"
/// No input completion suggestions are provided.
static member inline none = Interop.mkAttr "aria-autocomplete" "none"

@Zaid-Ajaj
Copy link
Owner

Thanks for sharing, I didn't see it there but (just as you said) I find it hard to come up with my own versions of names: inlinedSuggestions, inlined, inlineSuggestion etc.

@cmeeren
Copy link
Contributor Author

cmeeren commented Sep 1, 2019

Also, feel free to close this issue if you feel the final decision has been made on inheritFromParent vs. inherit'.

@Zaid-Ajaj
Copy link
Owner

Last comments on inheritFromParent before I close the issue

  • Doesn't use prime and backticks to make it look not so foreign (no F#-ers) for newcomers, especially inside online editors (github)
  • Reflects what it does in the name
  • Not used commenly so the long name will (hopefully) not discourage people from using the value too often when needed

As for ariaAutocomplete.inline' I will go for ariaAutocomplete.inlineAfterCarret because that is what the description says about the attribute.

Found a serious issue with this decision? Please open a new issue and let us discuss it 😄

@cmeeren
Copy link
Contributor Author

cmeeren commented Sep 10, 2019

Shouldn't it be inlineAfterCaret? (Single r)

@Zaid-Ajaj
Copy link
Owner

Sorry I wrote incorrectly here but added the right one (with single r) in the props

@zanaptak
Copy link
Contributor

Just pointing out that underscore is also an option to consider, either trailing or leading (e.g. inherit_, _inline). It's not as foreign as apostrophe since other languages and editing tools usually also allow underscores in identifiers.

I am not suggesting it for this issue. Decision to use word suffixes is perfectly fine. Just felt like an overlooked option to add to the discussion in case it helps any other onlookers.

@cmeeren
Copy link
Contributor Author

cmeeren commented Sep 11, 2019

Yep, though prefixing with underscore in F# usually means that it's an unused value (prefixing with underscore suppresses unused warnings).

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

3 participants