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

Enable use for plain ruby classes #91

Closed

Conversation

jsugarman
Copy link
Collaborator

@jsugarman jsugarman commented Jan 7, 2020

What

Allow creation of GDS design system form components for plain ruby classes. Issue.

Why

Inline with standard rails form builder helpers and to facilitate creation of simple forms with no models associated.

For example to create a simple search form it would be nice to not have to create any model and just do something like

# routes.rb
post 'search', action: :index, controller: :search
# app/controllers/search_controller.rb
class SearchController < ApplicationController
  def index
    @search_options = [[:opt1, 'Option 1'], [:opt2, 'Option 2']]
  end
end
# app/view/search/index.html.haml
= form_with url: search_path, local: true do |f|
  = f.govuk_collection_radio_buttons :search_option,
      @search_options,
      :first,
      :last,
      legend: { text: 'Choose a filter' },
      hint_text: 'Specify a filter for the search'
  = f.govuk_text_field(:query, label: { text: 'Find a thing' }, hint_text: 'enter thing to find' )
  = f.govuk_submit('Search')

Currently this is only prevented by the use of calls to errors, from what i can see.

allows `fdescribe, fcontext, fit` spec running
These are compiled/generated by yarn/npm install
and should not be in source control.
To test the formbuilder components use against
a plain ruby class.
Copy link
Member

@peteryates peteryates left a comment

Choose a reason for hiding this comment

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

Overall this looks good. Before this was raised I'd already created #90 which does more or less the same thing. If you're happy for me to I'll cherry pick the tests from this PR into that one and tweak accordingly. Thanks @jsugarman

@@ -65,6 +65,8 @@ def error_summary_title_id
end

def object_has_errors?
return unless @builder.object.respond_to?(:errors)
Copy link
Member

Choose a reason for hiding this comment

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

I think the builder should error if the dev tries to render an error summary in a form with an object that doesn't support errors

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Overall this looks good. Before this was raised I'd already created #90 which does more or less the same thing. If you're happy for me to I'll cherry pick the tests from this PR into that one and tweak accordingly. Thanks @jsugarman

Yep, what ever you think is best.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think the builder should error if the dev tries to render an error summary in a form with an object that doesn't support errors

👍 Makes sense, better to catch these things early.

spec/spec_helper.rb Show resolved Hide resolved
@peteryates peteryates closed this Jan 7, 2020
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.

None yet

2 participants