openstreetmap / openstreetmap-website

The Rails application that powers OpenStreetMap
https://www.openstreetmap.org/
GNU General Public License v2.0
2.2k stars 912 forks source link

Use a form builder for bootstrap forms #2654

Closed gravitystorm closed 3 years ago

gravitystorm commented 4 years ago

Our forms are quite varied, for various historical reasons. Some use the rails input helpers (e.g. f.text_field) and some are completely hand-crafted (with raw html, e.g. <input type=).

We also have a custom set of CSS styling rules for forms, some of which I dislike (e.g. making text inputs 50% wide, even on small screens) and some of which is confusing (e.g. our use of deemphasised buttons).

Finally, we have inconsistent behaviours between different forms, such as when and where to show error messages, location of cancel buttons (if any), marking dangerous actions with different button colours, and all these sort of things that people subliminally expect with modern websites.

So I'd like to move our forms to use Bootstrap styling, and gain all the nice things that come with it like hints, placeholders, buttons, error field highlights etc. But building those forms manually, even with rails helpers, involves a lot of work to get the errors and hints in the right places, and with the right CSS classes to get styled correctly. This is where simple_form comes in. I've used it, and other form builders like formtastic, in other projects, and simple_form ties in best with bootstrap. It greatly simplifies the form building and increases the chance of ending up on the "happy path" with errors hints etc all working smoothly.

I'm opening this issue just to explain what I'm planning, since this is another one of these things that will take multiple PRs to implement. I have some WIP code to share shortly, but here's a preview of the trace upload form. Screen Shot 2020-06-11 at 12 12 59

gravitystorm commented 4 years ago

So we have two options - #2655 (using the simple_form gem) and #2667 (using the bootstrap_form gem). Overall there's not much to choose between them, but as I said in #2667 I lean slightly towards the bootstrap_form option.

Today I've been tackling integrating the richtext input (that we use in diary entries and elsewhere). This would be nice to have as a custom input, so that we could do f.richtext_field :body and get the label, textarea, help, preview buttons etc automatically. After a bit of trial and error I found a way that works reasonably well.

Unfortunately bootstrap_form doesn't appear to have any way to develop custom inputs, either from scratch (e.g. f.richtext_field :body) nor by options (e.g. f.textarea :body, as: :richtextarea). I settled on creating a custom formbuilder, as in the linked commit, and it works well enough but still feels a bit clunky. The only relevant issue that I found was https://github.com/bootstrap-ruby/bootstrap_form/issues/139 . simple_form, as linked to in that issue, supports a number of nice methods to create custom inputs and other customisations.

@tordans do you have any experience with custom inputs like this?

Overall it's not a showstopper, so we can definitely move forward with bootstrap_form, but it would be nice to see if there's a smoother way to do the richtext fields.

tomhughes commented 4 years ago

I don't know if there's any overlap but I was already wondering if we should be looking at trying to migrate towards the built in richtext support in modern rails (the rich_text_area method).

gravitystorm commented 4 years ago

I don't know if there's any overlap but I was already wondering if we should be looking at trying to migrate towards the built in richtext support in modern rails (the rich_text_area method).

Yeah, I thought about that, but our three-way text/html/markdown situation meant that I decided to set that aside for the moment.

This custom input discussion and solutions will still be relevant for "locations" (e.g. users, diary_entries) where we store latitude and longitudes but use a map as the input - I don't see a location picker appearing in upstream any time soon!

tordans commented 4 years ago

@tordans do you have any experience with custom inputs like this?

Personally, no. But I asked a friend / dev at betterplace_org about it.

His reply (translated by me): simple_form is indeed a good fit for this use case. We used it to add custom color fields and a custom trix field. bootstrap_form is not ideal and not meant to be for this use case. Using a custom form builder is possible but not really great to work with (as you said, clunky). Maybe a simple partial will to the job for a while; but that depends on how often you use it and how much config you need.

gravitystorm commented 4 years ago

@tordans Thanks!

NishikantaRay commented 4 years ago

can i contribute for this

tomhughes commented 4 years ago

I'm not sure this is a good place for a newcomer to start @NishikantaRay especially as most of the simple cases have already been done and most of what remains involves some quite tricky problems. In particular I think most of the remaining ones require figuring out a solution to rich text inputs...

gravitystorm commented 3 years ago

Just as an update on this, many (most?) of the forms have been refactored in the last few months, and there's only a handful more old-style forms remaining.

gravitystorm commented 3 years ago

For the last few forms, I pushed on and reworked the CSS to use bootstrap, even if that meant that the more significant work to refactor them into using a form_builder was sidestepped. I intend to revisit the forms over time, particularly when refactoring controllers into more standard method names (a lot of the hand-built forms are mostly working around or fighting against the more natural way of doing rails forms).

With #3109 I've completed all the forms work, and removed all the 'standard-form' CSS from common.css