Skip to content

Match Button API to the other components#32

Merged
koppen merged 2 commits intomainfrom
button
Aug 26, 2025
Merged

Match Button API to the other components#32
koppen merged 2 commits intomainfrom
button

Conversation

@koppen
Copy link
Copy Markdown
Member

@koppen koppen commented Aug 22, 2025

All other components use a block to define their content, Button should as well.

All other components use a block to define their content, Button should
as well.
@koppen koppen requested a review from Copilot August 22, 2025 10:55
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR standardizes the Button component API to match other components by using block-based content definition instead of a label parameter. This improves consistency across the component library.

  • Removed the label parameter from the Button constructor
  • Updated the component to use block content via the content method
  • Updated all tests and preview code to use the new block-based syntax

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
app/components/flowbite/button.rb Removes label parameter and attr_reader, updates render method to use content
test/components/flowbite/button_test.rb Updates all test cases to use block syntax instead of label parameter
demo/test/components/previews/button_preview.rb Updates all preview examples to use block syntax instead of label parameter

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@koppen koppen requested review from RasmusDWN and olepalm August 23, 2025 20:26

def default
render(Flowbite::Button.new(label: "Default"))
render(Flowbite::Button.new) { "Default" }
Copy link
Copy Markdown
Contributor

@olepalm olepalm Aug 26, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For consistency's sake, that is the way to go. I still find it a bit odd that you cannot pass the (label) content to the initializer, like in an options or :label => {:content => "something") hash or similar.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It should probably look like Flowbite::Button.new(content: "something"), which I guess could be acceptable for the simplest case. The block style is well suited for the more complex cases as well, though, ie if you want more elements in your button:

<%= render(Flowbite::Button.new) do %>
  <div>
    <%= image_tag("icon") %>
    <%= translate(".submit") %>
  </div>
  <small><%= translate(".submit_details" %></small>
<% end %>

which would be cumbersome to do well in an argument.

@koppen koppen merged commit cc2e810 into main Aug 26, 2025
5 checks passed
@koppen koppen deleted the button branch August 26, 2025 09:03
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

Successfully merging this pull request may close these issues.

3 participants