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

Refactor to use class_names helper #587

Closed
3 tasks
joemasilotti opened this issue Jul 28, 2022 · 7 comments
Closed
3 tasks

Refactor to use class_names helper #587

joemasilotti opened this issue Jul 28, 2022 · 7 comments
Assignees
Labels
help wanted Looking for help from the community

Comments

@joemasilotti
Copy link
Owner

joemasilotti commented Jul 28, 2022

I recently learned about the class_names helper from @swanson's blog post on Hotwire-related CSS tricks.

I'd love to leverage this to remove all string interpolation of class names across the codebase.

A PR addressing this issue should:

  • Remove all stripe interpolated class name logic
  • Replace said logic with the class_names helper
  • See if there are any additional abstractions we can pull out.
@joemasilotti joemasilotti added help wanted Looking for help from the community dev UX labels Jul 28, 2022
@joshio1
Copy link
Contributor

joshio1 commented Jul 29, 2022

Hey @joemasilotti, can I take a look at this one?

@joemasilotti
Copy link
Owner Author

Sure thing - all yours!

@joshio1
Copy link
Contributor

joshio1 commented Aug 1, 2022

Hi @joemasilotti, while I' looking into this one, I'm not 100% sure which one you meant when you said "stripe interpolated class name logic".
I tried searching for classes in the app/views but couldn't find any interpolation for class names. Inside the app/components folder, there were a few places which had class name interpolation but I'm not sure if those were the ones you meant. These are the files I found which had class name interpolation:

  1. toast_message_component.html.erb#13
  2. avatar_component.html.erb#5
  3. cover_image_component.html.erb#2
  4. sort_button_component.html.erb#1
  5. submit_button_component.html.erb#1
  6. search_status_component.html.erb#4

@joemasilotti
Copy link
Owner Author

Yep! Those are exactly what I'm talking about. I think a perfect example is SortButtonComponent#dynamic_classes - ideally those classes live in the HTML, not the Ruby file. So app/components/sort_button_component.html.erb:1 could look like:

<%= button_tag title, form: form_id, name: name, value: value, type: :submit, class: class_names: "block w-full text-left px-4 py-2 text-sm cursor-pointer hover:bg-gray-100", "font-medium text-gray-900": active?, "font-normal text-gray-700": !active? %>

This wasn't checked, but I think we can pass multiple class names in like that. Let me know if that doesn't make sense (or doesn't work!).

@joshio1
Copy link
Contributor

joshio1 commented Aug 2, 2022

Hey @joemasilotti, so I've raised this PR: #593

I've used the new style in card_component.html.erb, search_status_component.html.erb, sort_button_component.html.erb but not in other files like toast_component.html.erb, avatar_component.html.erb, cover_image_component.html.erb submit_button_component.html.rb. Please let me know if it needs to be modified in any of the other files.

Also, do you think we should add any tests for this? If yes, which tests?

@joemasilotti
Copy link
Owner Author

Awesome, thanks! I'll check it out. I'll comment in the PR about tests and such.

@joemasilotti
Copy link
Owner Author

Closed via #593.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Looking for help from the community
Projects
Status: In progress
Development

No branches or pull requests

2 participants