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

Use dig in layouts guide ERB #629

Merged
merged 1 commit into from
Jul 11, 2024
Merged

Conversation

neanias
Copy link
Contributor

@neanias neanias commented Jun 24, 2024

If one uses the suggested layout for the ViewComponent default layout, navigating to localhost:3000/rails/view_components will throw an error due to the deeply nested nature of the Lookbook params hash. Using dig allows for safe access into the hashes without causing problems if any of the keys don't exist.

Hash#dig was introduced in Ruby 2.3, so will be available in all supported versions of Lookbook.

There's a few other instances of params[:some][:deep][:key] in the docs, but I thought this one may be a better target since it could cross the Lookbook boundary to ViewComponent.

@neanias neanias changed the title docs: use dig in layouts guide ERB Use dig in layouts guide ERB Jun 24, 2024
Copy link

netlify bot commented Jun 24, 2024

Deploy Preview for lookbook-docs ready!

Name Link
🔨 Latest commit 56f35dd
🔍 Latest deploy log https://app.netlify.com/sites/lookbook-docs/deploys/668bf8df8cf40d000846c76c
😎 Deploy Preview https://deploy-preview-629--lookbook-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@Spone
Copy link
Contributor

Spone commented Jun 24, 2024

Good catch @neanias! Thanks for the PR.

About the docs, I'd stay it makes sense to improve the syntax there as well, what do you think @allmarkedup?

@neanias
Copy link
Contributor Author

neanias commented Jun 24, 2024

I'm happy to change the others as part of this PR if that helps the sweep?

@allmarkedup
Copy link
Collaborator

@neanias @Spone sorry about the slow reply! This is a good catch, thank you.

@neanias if you would be willing to update the other places that this syntax is used in the docs that would be fantastic, thank you. If you add it to this PR then I'll merge once ready. Appreciate your time on this!

If one uses the suggested layout for the ViewComponent default layout,
navigating to `localhost:3000/rails/view_components` will throw an error
due to the deeply nested nature of the Lookbook params hash. Using `dig`
allows for safe access into the hashes without causing problems if any
of the keys don't exist. This sweeps other instances of deep hash
access.

Hash#dig was introduced in Ruby 2.3, so will be available in all
supported versions of Lookbook.
@neanias
Copy link
Contributor Author

neanias commented Jul 8, 2024

I think that's the main ones. I found page.data[:brand_colors][:red] but I think that one's ok?

@Spone Spone requested a review from allmarkedup July 9, 2024 05:17
@allmarkedup
Copy link
Collaborator

I think that's the main ones. I found page.data[:brand_colors][:red] but I think that one's ok?

@neanias yep that should be fine I think. Many thanks for your time with this, will merge now 👍

@allmarkedup allmarkedup merged commit d35a96c into lookbook-hq:main Jul 11, 2024
16 checks passed
@neanias neanias deleted the patch-1 branch July 15, 2024 13:50
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.

3 participants