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

Button - add additional element support? #70

Closed
johlrich opened this issue Oct 11, 2017 · 8 comments
Closed

Button - add additional element support? #70

johlrich opened this issue Oct 11, 2017 · 8 comments

Comments

@johlrich
Copy link
Contributor

Ran into this because I was expecting Button.button to be a button element when rendered, not an a.

Since Bulma can style buttons using a, button, and input type="button|submit|reset", should there be Button.button, Button.button_a, Button.button_input like some of the others components that have multiple ways of being represented?

@MangelMaxime
Copy link
Collaborator

I think it would make sense to use have:

  • Buton.button_div
  • Buton.button_a

We can probably add Buton.button_input too but I don't really see where you want to use them with Elmish. Because, their nice for form when doing classic web application as they can trigger form submit by them self. But in Elmish you never do that. Also, even if we add Buton.button_input, I will not add Button.submit for example because it can't be applied to a div element.

@johlrich
Copy link
Contributor Author

Ok I'll get a PR going later to add these and get some feedback.

I hadn't heard of it being on a div, according to the Bulma docs. I was just going to make Button.button use the button element and Button.button_a use the a element

The .button class can be used on:

a> anchor links
button> form buttons
input type="submit"> submit inputs
input type="reset"> reset inputs

I get the hesitance for submit since it can only go on a button and users could use the intellisense to think they can put it on any of the button types (like I did when I discovered it's being rendered as an anchor element atm 😄 )

@MangelMaxime
Copy link
Collaborator

Sure feel free to open a PR and yes if we specialize one helper with _element better to not have a default one because you will never remember instantly which element is behind.

@MangelMaxime
Copy link
Collaborator

Ok, so I am adding:

  • Button.button_a
  • Button.button_div

But not Button.button_input because an input can't have any children, so it's not the same DSL etc.. Also IMO, the only benefit for using button of type input, is to use the default feature attach to a form like auto form submit and this doesn't play well with Elmish concept.

If you really need it, feel free to re-open to discuss it further.

@johlrich
Copy link
Contributor Author

What about leaving Button.button as the button element? So Button.button [Button.isPrimary] [str "Click"] would give <button class="button is-primary">Click</button>.

As an aside, I'm not sure what value button_div has tbh, especially when the bulma docs don't even mention this as an option, but if it works I guess no big deal there.

Sorry I didn't get a PR last week, things kept coming up. I have an almost completed PR for Steps extension almost done too, just need to fix up docs.

@MangelMaxime
Copy link
Collaborator

MangelMaxime commented Oct 17, 2017

In reality, you can use the button class to a span, div, input, a, etc. but yes according to the documentation:

The .button class can be used on:

anchor links
form buttons
submit inputs
reset inputs
Anchor

I wanted to add the div element, because if we use the a element we should provide the href attribute to it...

Again, if we provide one custom function Button.button_.... I don't wan't to provide a default one because it's not direct which element will be behind. I know, it's not that direct neither here but could be worse if we start having custom elements and default one everywhere.

Again, what's the benefit of having an input or button element as the button for you with Elmish ? Can you share as case where a a button can't ?

Here the visual display is the same. When I provided custom helpers at the time it's was because there was some really small change when using one or another version. But I think, it's probably no more the case and it's kind of something I regret because it's add complexity which is not needed.

Edit: For the next version of Fulma, I will revert my change on the button element (it's better :) ). And, I will explain in the documentation that's we only genera the a element. To make it clear in the documentation perhaps that's a good compromise because at first your confusion comes because you was thinking of it as button/input element.

@johlrich
Copy link
Contributor Author

johlrich commented Oct 17, 2017

My main reasoning is when using Fulma without Elmish. Button isn't in Fulma.Elmish and there are non-Elmish use cases that Fulma is a great fit for as well. Main use case is being able to add type="submit". Even if the DSL doesn't encourage the submit type, Fulma can enable the form submission use case without having additional helpers by adding a custom prop via the existing apis.

For my case, even though I am using Elmish, I actually prefer the form submission approach for most of my forms there too. Instead of firing Msgs for each and every keystroke / input change, I'd rather tie into the form's onchange/onsubmit to do validation + group up the data to cut down on the amount of plumbing I need in the application state components. Instead, the plumbing stays in the View and I only dispatch application messages when form's validation state changes or a valid form is submitted.

@MangelMaxime
Copy link
Collaborator

Hum, ok.
It's kind of a bad idea to not store your application state inside your model but if you like it and I know some others people do prefer this way too I will adapt Fulma.

Also, as Fulma can be used to render static web site it can make sense.

So, I will provide:

  • Button.button_a
  • Button.button_btn
  • Button.button_input

Like so, we are supporting the same scope as Bulma documentation.

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

No branches or pull requests

2 participants