Improve theme selection #2152
Reference in New Issue
Block a user
Delete Branch "improved-theme-selection"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Currently when selecting a theme you have to hit the radio button directly - you can't choose the image or the text. I've been swapping back and forth quite a bit and found this a bit annoying so decided to try and make this better! This PR now allows you to select the entire 'area' for a given theme option. I also hid the radio button itself (except for screen readers) to make it look a little nicer.
Before and after (updated):
https://github.com/user-attachments/assets/14eb48df-f298-478e-b755-744d5769f1ac
One thing I don't quite know - - there's a bit of a delay with the selection compared to the current implementation. I'm definitely not a ruby expert so maybe something isn't ideal here, or it's just the addition of a controller itself. Happy to get feedback!This switching is instant by using HTML form labels and the existing auto-submit controller.Agree with this idea!
Just to check, @justinfar what are your thoughts around removing checkboxes here? I'm cool with either.
@@ -45,2 +45,2 @@<div><%= styled_form_with model: @user, class: "flex flex-col md:flex-row justify-between items-center gap-4", data: { controller: "auto-submit-form" } do |form| %><div data-controller="theme" data-theme-user-preference-value="<%= @user.theme %>"><%= form_with model: @user, class: "flex flex-col md:flex-row justify-between items-center gap-4", id: "theme_form",Just to stay consistent with the rest of the codebase, I think we probably want to keep this existing auto-submit controller wired up here.
I agree with your idea of wrapping those radio buttons though so clicks can be captured anywhere on the image or radio input.
Can we find a way to re-structure the HTML with a form label of some sort?
I think changing from
styled_form_withto a regularform_withto avoid all those default form styles might help quite a bit here and give you more access to the inputs directly.@@ -45,2 +45,2 @@<div><%= styled_form_with model: @user, class: "flex flex-col md:flex-row justify-between items-center gap-4", data: { controller: "auto-submit-form" } do |form| %><div data-controller="theme" data-theme-user-preference-value="<%= @user.theme %>"><%= form_with model: @user, class: "flex flex-col md:flex-row justify-between items-center gap-4", id: "theme_form",This was great feedback thanks! I changed a bunch of stuff up here - it's now using the
form_withand html form labels. I've removed the theme specific controller as well so that's nice. I also swapped some other stuff around to remove a bunch of code and iterate over a list. As a part of all of this (I think relying directly on the form labels), the performance of all of it is nearly instant and it feels very smooth:https://github.com/user-attachments/assets/85612d83-efe9-490f-affc-f30df332beda
Nice work here! This looks great.
@@ -45,2 +45,2 @@<div><%= styled_form_with model: @user, class: "flex flex-col md:flex-row justify-between items-center gap-4", data: { controller: "auto-submit-form" } do |form| %><div data-controller="theme" data-theme-user-preference-value="<%= @user.theme %>"><%= form_with model: @user, class: "flex flex-col md:flex-row justify-between items-center gap-4", id: "theme_form",Looks great!