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

all caps for HTML components? #120

Open
catmando opened this issue Jan 21, 2016 · 6 comments
Open

all caps for HTML components? #120

catmando opened this issue Jan 21, 2016 · 6 comments

Comments

@catmando
Copy link
Collaborator

thinking that we should allow HTML components to be expressed in all CAPS.

class TodoList < React::Component::Base

  param :items, type: [String]

  def render
    UL do
      params.items.each_with_index do |item, index|
        LI (key: "item - #{index}") { item }
      end
    end
  end
end

class TodoApp < React::Component::Base

  before_mount do
    state.items! []
    state.text! ""
  end

  def render
    DIV do
      H3 { "TODO" }
      TodoList items: state.items
      DIV do
        INPUT(value: state.text).on(:change) do |e|
          state.text! e.target.value
        end
        BUTTON { "Add ##{state.items.length+1}" }.on(:click) do |e|
          state.items! (state.items + [state.text!("")])
        end
      end
    end
  end
end

Element["#todo-target"].render { TodoApp() }

this seems to make the code a lot more readable, and distinguishes between builtin tags (ALL CAPS), application defined components (CamelCase) and other methods (snake_case).

The downside is that its harder to type, although its probably easy to program your favorite editor to do the upcase automatically.

This came up in a discussion with @loicboutet btw.

@fkchang
Copy link
Contributor

fkchang commented Jan 22, 2016

I'm not certain I like how it looks, I'd probably have to use it for a while to decide. That being said, if it's "just allow" i.e. in addition to the lowercased stuff, I'm fine w/adding it. From haml/slim, I'm more used to lowercase

@ajjahn
Copy link
Collaborator

ajjahn commented Jan 22, 2016

This doesn't really suit my tastes, so I can't see myself using it. I prefer ruby's convention of uppercase being reserved for constants. In my opinion, if a component is growing to the point that it is difficult to read, it's time to break it into small components.

@loicboutet
Copy link

I would say having way to see what is a html tag and is a ruby function would be really helpful.
I agree that full caps looks slightly weird, I would prefer if there was another way, but I don't find any. All in all I personally think that it would be worth it at the end of day to adopt this style.

@catmando
Copy link
Collaborator Author

@ajjahn - I think the concern is not with the size of the component. In fact this came up when reading a very small component... The problem is to distinguish between regular old methods, (params or my_private_method for example) HTML tags (div, etc) and application defined components (Clock).

As far as constants go, well in a very real sense the HTML tags are constants, so it sort of makes sense.

@fkchang - while we would not want to deprecate this (after all catprint already has a huge pile of code to maintain) the question is more to do with standards, and how we will present code in tutorials, etc.

@fkchang
Copy link
Contributor

fkchang commented Jan 25, 2016

@catmando I lean towards it as optional, these are all html tags, right, so I sort of expect a web developer to have familiarity with them, and if they are Rails programmers, it's likely they have been exposed to haml and slim, so the DSL would resemble those. I do sort of share @ajjahn's concerns a bit, in that what I like about the ruby DSL is that while it might look like slim (or like markaby nearly exactly), it's in fact just Ruby.

@sollycatprint
Copy link

This issue was moved to ruby-hyperloop/hyper-react#120

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