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.

Sign up for GitHub

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

Jump to bottom

Inline component variant bug #429

Merged
merged 2 commits into from
Oct 29, 2020
Merged

Inline component variant bug #429

merged 2 commits into from
Oct 29, 2020

Conversation

mrjonesbot
Copy link
Contributor

@mrjonesbot mrjonesbot commented Jul 29, 2020 •
edited
Loading

Resolves #319

Summary

While rendering inline components without template files, unable to render variant call methods -- eg. call_email -- using the #render :variants option.

Non working example:

render(InlineComponent.new, variants: [:email])

Changes

Introduces the #with_variant instance method, which sets the variant to be rendered inline (with and without template).

@mrjonesbot mrjonesbot mentioned this pull request Jul 29, 2020
Closed
@mrjonesbot
Copy link
Contributor Author

@BlakeWilliams Did a first pass of #with_variant, seems straightforward but let me know if there's more to it. Thanks!

@mrjonesbot mrjonesbot force-pushed the inline-component-variant-bug branch 4 times, most recently from c32ade1 to c8a1656 Compare August 15, 2020 15:50
@stale
Copy link

stale bot commented Sep 20, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Sep 20, 2020
@joelhawksley joelhawksley added active and removed stale labels Sep 21, 2020
@joelhawksley
Copy link
Member

@mrjonesbot mind rebasing this PR? You can remove coverage.txt, as it's been removed in master.

@mrjonesbot
Copy link
Contributor Author

@joelhawksley done!

@mrjonesbot mrjonesbot force-pushed the inline-component-variant-bug branch 2 times, most recently from 46fda84 to dcf2db0 Compare September 24, 2020 15:03
@mrjonesbot mrjonesbot force-pushed the inline-component-variant-bug branch 4 times, most recently from 8969809 to ef0f459 Compare October 28, 2020 16:53
joelhawksley
joelhawksley approved these changes Oct 29, 2020
View reviewed changes
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.

Looks good to me! Mind adding a CHANGELOG entry?

@joelhawksley
Copy link
Member

@mrjonesbot sorry for the extra cycle, but could you also add a note about this functionality to docs/index.md?

@mrjonesbot
Render inline components #with_variant
efc909d 
Resolves ViewComponent#319

Introduces the `#with_variant` instance method, which sets the variant
to be rendered inline (with and without template).

Show usage in README.md
joelhawksley
joelhawksley reviewed Oct 29, 2020
View reviewed changes
docs/index.md Outdated Show resolved Hide resolved
mrjonesbot
mrjonesbot commented Oct 29, 2020
View reviewed changes
docs/index.md
@@ -325,6 +325,14 @@ class InlineVariantComponent < ViewComponent::Base
end
```

And render them `with_variant`:
Copy link
Contributor Author

@mrjonesbot mrjonesbot Oct 29, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

Ah right this is MD 😄

@joelhawksley joelhawksley merged commit ba5cb1e into ViewComponent:master Oct 29, 2020
@joelhawksley
Copy link
Member

Thank you for your contribution, Nathan! ❤️

@mrjonesbot mrjonesbot deleted the inline-component-variant-bug branch October 30, 2020 15:42
@joelhawksley joelhawksley mentioned this pull request Jun 9, 2021
Closed
@joelhawksley
Copy link
Member

@mrjonesbot looking back, can you share the context for why you wanted to have this feature added? We're wondering why you'd want to override a variant in a template.

@mrjonesbot
Copy link
Contributor Author

mrjonesbot commented Aug 16, 2021 •
edited
Loading

@mrjonesbot looking back, can you share the context for why you wanted to have this feature added? We're wondering why you'd want to override a variant in a template.

Hey Joel, I think I was trying to learn the library and was attempting a pattern shown in the README at the time; then noticed it didn't work as described.

Not sure I had a personal use case for it, can't remember now.

@joelhawksley joelhawksley mentioned this pull request Aug 16, 2021
Closed
@CoderMiguel CoderMiguel mentioned this pull request Jun 30, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@joelhawksley joelhawksley joelhawksley approved these changes

@BlakeWilliams BlakeWilliams Awaiting requested review from BlakeWilliams

Assignees
No one assigned
Labels
active
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Unable to render Inline Component Variants
3 participants
@mrjonesbot @joelhawksley @BlakeWilliams