Skip to content

Adds documentation for usage with Rails forms#946

Closed
pinzonjulian wants to merge 6 commits intoViewComponent:mainfrom
pinzonjulian:docs/forms
Closed

Adds documentation for usage with Rails forms#946
pinzonjulian wants to merge 6 commits intoViewComponent:mainfrom
pinzonjulian:docs/forms

Conversation

@pinzonjulian
Copy link

@pinzonjulian pinzonjulian commented Jun 3, 2021

Closes #908

Summary

This PR adds a comprehensive guide to using ViewComponent with forms in Ruby on Rails.

Notable changes

  1. I decided to add a new section to the documentation called Practical Examples as this guide is more of an explanation on how to use ViewComponent for a particular purpose rather than how to use the library itself.
  2. I've removed the forms issue from the Known Issues section and included it inside the guide for forms.

Other Information

I wrote this guide with an opinionated tone as if these were a set of "good practices" when using ViewComponent in the context of Rails forms. The advice in this guide comes from having used this library in place of ActionView for the past year and a half and finding the best way possible to let each part of the framework do what they do best. However these are just the opinions from my experience so any feedback from other heavy ViewComponent users is super welcome!

Navigation screenshot with new section

image

Snippet of table of contents

For context when reviewing this PR

image

@pinzonjulian pinzonjulian marked this pull request as ready for review June 3, 2021 21:02
@pinzonjulian pinzonjulian mentioned this pull request Jun 3, 2021
2 tasks
@Spone
Copy link
Collaborator

Spone commented Jun 4, 2021

Thanks @pinzonjulian! I think the "Practical Examples" section is a good idea. I added a few comments and fixes :)

@pinzonjulian
Copy link
Author

@Spone @JoshTeperman @MatheusRich Thanks for your review! I merged all of your grammar and typo suggestions. I also restructured and rewrote the section about form builders and it's a bit clearer now. Let me know what you think.

### Using multiple `FormBuilder`s in your application
In a typical Rails application, all views rendered by controllers that inherit from your `ApplicationController` will use the default `FormBuilder` provided by the framework. However, you can override the default `FormBuilder` used by your application wherever you think necessary. We previously saw that you can do so using the `builder` argument in `form_with` but you can also configure it at a [`controller` level](https://edgeapi.rubyonrails.org/classes/ActionController/FormBuilder.html){:target="_blank"}.

This is currently *not supported* by `ViewComponent`. To overcome this, apply the correct form builder each time to the `form_with` method.
Copy link
Contributor

Choose a reason for hiding this comment

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

What part of this is not supported by ViewComponent? Do you have an example of something that doesn't work? I have a custom form builder in my application, and it renders components similar to how you have shown above. I set the form builder in my ApplicationController and it works fine. To me this is just the way Rails works. I'm not understanding what setting a form builder has to do with ViewComponent at all.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for raising this @davekaro. I used this a few months ago and it didn't work correctly for me but maybe things have changed since then. I removed this section for now.

I'll test this with the current version of ViewComponent to make sure it's actually an issue and update the documentation in a separate PR if necessary. I think I jumped the gun there 🙈. Sorry and thanks for pointing it out!

Update docs/practical-examples/forms.md

Co-authored-by: Josh Teperman <36095443+JoshTeperman@users.noreply.github.com>

Update docs/practical-examples/forms.md

Co-authored-by: Josh Teperman <36095443+JoshTeperman@users.noreply.github.com>

Update docs/practical-examples/forms.md

Co-authored-by: Josh Teperman <36095443+JoshTeperman@users.noreply.github.com>

Update docs/practical-examples/forms.md

Co-authored-by: Josh Teperman <36095443+JoshTeperman@users.noreply.github.com>

Update docs/practical-examples/forms.md

Co-authored-by: Hans Lemuet <Spone@users.noreply.github.com>

Update docs/practical-examples/forms.md

Co-authored-by: Hans Lemuet <Spone@users.noreply.github.com>

Update docs/practical-examples/forms.md

Co-authored-by: Hans Lemuet <Spone@users.noreply.github.com>

Update docs/practical-examples/forms.md

Co-authored-by: Hans Lemuet <Spone@users.noreply.github.com>
@pinzonjulian
Copy link
Author

Thanks to everyone's comments and suggestions. I believe the current shape of the docs for forms is stable. Thanks for your help and let's wait for it to be reviewed and merged.

@joelhawksley joelhawksley requested a review from Spone June 7, 2021 15:49
@pinzonjulian
Copy link
Author

Hey! Friendly bump to get this one in. Is there anything preventing it from being merged?

@Spone
Copy link
Collaborator

Spone commented Jul 20, 2021

Sorry about the delay @pinzonjulian. Do you mind adding a changelog entry?

@joelhawksley
Copy link
Member

@pinzonjulian sorry for the delay in getting you a review! I was holding off on providing feedback here due to the ongoing discussion in https://github.com/github/view_component/discussions/420.

That being said, I'll give this some 👀 today (if you want to hold off on merging for now, @Spone ❤️ )

Copy link
Member

@joelhawksley joelhawksley left a comment

Choose a reason for hiding this comment

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

👋🏻 thanks for taking the time to write this documentation! I have some thoughts on how to tighten things up a bit ❤️

I've only reviewed a portion of the document, figuring that you might make other changes based on the feedback provided.

## Compatibility with Rails Forms

ViewComponent is [not currently compatible](https://github.com/github/view_component/issues/241) with `form_for` helpers.
ViewComponent works for most cases using form helpers in Rails. See the [forms](practical-examples/forms.md) guide in the Practical Examples section for more information on compatibility.
Copy link
Member

@joelhawksley joelhawksley Jul 20, 2021

Choose a reason for hiding this comment

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

Suggested change
ViewComponent works for most cases using form helpers in Rails. See the [forms](practical-examples/forms.md) guide in the Practical Examples section for more information on compatibility.
ViewComponent works with most Rails form helpers. See the [forms](practical-examples/forms.md) guide for more information on compatibility.


## Before you start

It's important to understand the building blocks of forms in Ruby on Rails before attempting to use `ViewComponent` to enhance them.
Copy link
Member

Choose a reason for hiding this comment

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

Do we need to expect folks to have this context to use forms with ViewComponents? I worry that this could scare people off. I'd prefer that we move this section to a reference section at the bottom of this page.


## Using form helpers inside a `ViewComponent`

If your component needs to render a form, you can use the `form_with` helpers just as you would in a Rails template or partial
Copy link
Member

Choose a reason for hiding this comment

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

As a general rule, our documentation style guidelines recommend avoiding you.

Suggested change
If your component needs to render a form, you can use the `form_with` helpers just as you would in a Rails template or partial
To render a ViewComponent with a form, use the `form_with` helpers just as in a Rails template or partial:

---
layout: default
title: Forms
parent: Practical Examples
Copy link
Member

Choose a reason for hiding this comment

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

For now, I think we should put this in the existing Guide. I'm not confident that we have a good framework for delineating between two different types of content and I think that most folks will find this document useful ❤️

def initialize(comment:)
@comment = comment
end
attr_reader :comment
Copy link
Member

Choose a reason for hiding this comment

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

In the interest of keeping our examples as simple as possible, IMO we should leave this line out.

Suggested change
attr_reader :comment

Comment on lines +73 to +75
# The name of the folder doesn't really matter as long as it's inside
# your app folder for rails to autoload it.

Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# The name of the folder doesn't really matter as long as it's inside
# your app folder for rails to autoload it.

# The name of the folder doesn't really matter as long as it's inside
# your app folder for rails to autoload it.

class YourCustomFormBuilder < ActionView::Helpers::FormBuilder
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
class YourCustomFormBuilder < ActionView::Helpers::FormBuilder
class CustomFormBuilder < ActionView::Helpers::FormBuilder

💡 To understand how `FormBuilder` works, it's a good idea to see how the [default class](https://github.com/rails/rails/blob/main/actionview/lib/action_view/helpers/form_helper.rb){:target="_blank"} in Rails defines helpers

```ruby
# app/form_builders/your_custom_form_builder.rb
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
# app/form_builders/your_custom_form_builder.rb
# app/form_builders/custom_form_builder.rb

end
```

To use `YourCustomFormBuilder` you can either define it in your controllers or directly as an argument to `form_with`. In this example, we'll pass it as an argument to `form_with`
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
To use `YourCustomFormBuilder` you can either define it in your controllers or directly as an argument to `form_with`. In this example, we'll pass it as an argument to `form_with`
To use `YourCustomFormBuilder` either define it in a controller or directly as an argument to `form_with`. In this example, it's passed as an argument to `form_with`:

<% end %>
```

### Using View Components
Copy link
Member

Choose a reason for hiding this comment

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

Using ViewComponents for what?

```

```erb
# fields.html.erb

Choose a reason for hiding this comment

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

Suggested change
# fields.html.erb
# _fields.html.erb

@Spone
Copy link
Collaborator

Spone commented Sep 17, 2021

@pinzonjulian let me know if you need help wrapping this up, we can take some time to pair on it!

@pinzonjulian
Copy link
Author

pinzonjulian commented Oct 9, 2021

@pinzonjulian let me know if you need help wrapping this up, we can take some time to pair on it!

Hey @joelhawksley and @Spone. Sorry for taking long to respond but I didn't have enough time to get back into this.

I'd love to push through with this topic but coming back to this with fresh eyes, this PR seems much more like my personal opinion rather than an official take from the core team on a crucial topic for any app using server-side rendered Rails that is handling forms.

Before continuing I'd love to come to a consensus about this topic. I've expressed my opinions in detail in this PR and in https://github.com/github/view_component/discussions/420 but haven't really heard from core. There are things in this discussion about modifying rails to be able to use it to VC's advantage but without an opinion from you it's hard to push those ideas further.

I know this is a hard topic and that there are still things to iron out like #974 but I believe that it's very important to have a proper stance from you to bring confidence to users about this topic so if this needs to wait until there's a full solution, I'd prefer to wait.

@Spone
Copy link
Collaborator

Spone commented Oct 11, 2021

I'd be happy to discuss this in more details with you @pinzonjulian, @joelhawksley & others!

@pinzonjulian
Copy link
Author

Happy to as well!

@joelhawksley
Copy link
Member

Closing as stale ❤️

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.

Add forms section to guide

7 participants