Skip to content

Comments

Action View Overview Guide#4

Open
bhumi1102 wants to merge 44 commits intomainfrom
bhumi-guides-action-view-overview
Open

Action View Overview Guide#4
bhumi1102 wants to merge 44 commits intomainfrom
bhumi-guides-action-view-overview

Conversation

@bhumi1102
Copy link
Owner

@bhumi1102 bhumi1102 commented Feb 28, 2024

Action View Overview is ready for review. I'm breaking this bigger task of Action View + Layouts and Rendering guide + new guide into multiple steps. This is step 1. I've gone through Action View Overview top to bottom.

Another PR upcoming for Layout and Rendering guide.

Here are the bullet points from the to-do:

  • Intro about using Action View seems a bit convoluted
  • We could make a better connection that "templates" are most of the time referred to as simply "views"
  • Templates could do a better job explaining what the .html.erb are, or linking to the Layouts & Rendering guide.
  • Could link to the builder gem in that section.
  • There's a brief mention of template caching, that could be expanded.
  • Added a little bit about Fragment caching and Shared Partial caching and links to the Caching with Rails guide. Can you think of more about caching?
  • There's a section on partials on both guides.
  • Merged both into one place. For now all partials related subsections are in this Action View Overview guide. (open and thinking about to a different division tbd)
  • The intro section to layouts just mention it and delegates to the other guide, but the actual content on layouts is very bare.
  • View Paths is a section that is probably better suited for a more advanced guide, perhaps in layouts and rendering instead of the overview. (it's not something people usually mess with, and it's less about action view and more about action controller -> view)
  • The I18n locale example should likely use an around_filter, like the one in the I18n guide. (generally safer to reset that state with an around_block)
  • The localized views section explains about using a non-standard locale "expert" to force views lookup to that, but that's probably incorrect now, I18n complains about invalid locales. We also have other ways of doing it now with variants for instance. (which aren't explained anywhere apparently)
  • ah okay. I have not had an occasion to implement I18n with Rails, so not super familiar with this one.

Copy link
Collaborator

@carlosantoniodasilva carlosantoniodasilva left a comment

Choose a reason for hiding this comment

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

Looking good @bhumi1102! I had some initial suggestions below.

I know there's probably going to have some back-and-forth as you work through the layouts & rendering, let me know if I can help with anything there.

Copy link
Collaborator

@Ridhwana Ridhwana left a comment

Choose a reason for hiding this comment

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

This looks good ✨ Left some very minor nitpicks/suggestions.

@bhumi1102 bhumi1102 changed the title Bhumi guides action view overview Action View Overview Guide Mar 26, 2024
Copy link
Collaborator

@carlosantoniodasilva carlosantoniodasilva left a comment

Choose a reason for hiding this comment

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

Updates are looking great @bhumi1102! I had just a few minor bits of feedback on a last pass, otherwise this lgtm.

bhumi1102 and others added 20 commits March 28, 2024 09:22
Co-authored-by: Carlos Antonio da Silva <carlosantoniodasilva@gmail.com>
Co-authored-by: Carlos Antonio da Silva <carlosantoniodasilva@gmail.com>
Co-authored-by: Carlos Antonio da Silva <carlosantoniodasilva@gmail.com>
Co-authored-by: Carlos Antonio da Silva <carlosantoniodasilva@gmail.com>
Co-authored-by: Carlos Antonio da Silva <carlosantoniodasilva@gmail.com>
Co-authored-by: Carlos Antonio da Silva <carlosantoniodasilva@gmail.com>
Co-authored-by: Carlos Antonio da Silva <carlosantoniodasilva@gmail.com>
Co-authored-by: Petrik de Heus <petrik@deheus.net>
Co-authored-by: Matheus Richard <matheusrichardt@gmail.com>
Co-authored-by: Petrik de Heus <petrik@deheus.net>
Co-authored-by: Alan Savage <asavageiv@users.noreply.github.com>
Co-authored-by: Petrik de Heus <petrik@deheus.net>
Co-authored-by: Erick Sasse <148989+esasse@users.noreply.github.com>
Co-authored-by: Carlos Antonio da Silva <carlosantoniodasilva@gmail.com>
Co-authored-by: Carlos Antonio da Silva <carlosantoniodasilva@gmail.com>
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.

4 participants