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

D8CORE-1632: Add variables to quote template #678

Merged
merged 2 commits into from
May 27, 2020
Merged

D8CORE-1632: Add variables to quote template #678

merged 2 commits into from
May 27, 2020

Conversation

sherakama
Copy link
Member

READY FOR REVIEW

Summary

  • Adds new variables to quote template to allow for different media options

Review By (Date)

  • May 1st?

Urgency

  • Low

Steps to Test

  1. Review code
  2. Review render in build url

Affected Projects or Products

  • Does this PR impact any particular projects, products, or modules?

Associated Issues and/or People

  • D8CORE-1632
  • SOE Person type

See Also

@jenbreese
Copy link
Contributor

@sherakama can you restart the tugboat? Or are we totally over the quota?

yvonnetangsu
yvonnetangsu reviewed Apr 30, 2020
View reviewed changes
core/src/templates/components/quote/quote.twig
{%- if src is not empty -%}
<img class="su-quote__img" src="{{ src }}" alt="{{ alt }}">
{%- endif -%}
{%- block block_quote_media -%}
Copy link
Member

Choose a reason for hiding this comment

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

Actually could this use the nested media.twig template, like what we did for the Card component?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh, that might be an option. Let me see about that refactor.

yvonnetangsu
yvonnetangsu reviewed Apr 30, 2020
View reviewed changes
Copy link
Member

@yvonnetangsu yvonnetangsu left a comment

Choose a reason for hiding this comment

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

Just that question about nested media.twig, and if not, do we need any stying for {{ quote_media }}, eg, float:left?

@sherakama
Copy link
Member Author

Just that question about nested media.twig, and if not, do we need any stying for {{ quote_media }}, eg, float:left?

Naw, I'd like this to be an open field, and developers can bring their own styles if they add something wacky to it.

@yvonnetangsu
Copy link
Member

Just that question about nested media.twig, and if not, do we need any stying for {{ quote_media }}, eg, float:left?

Naw, I'd like this to be an open field, and developers can bring their own styles if they add something wacky to it.

@sherakama Please feel free to merge this in first so you can use the open {{ quote_media }} field. We can always decide whether we want the nested media.twig later.

@sherakama
Copy link
Member Author

@sherakama Please feel free to merge this in first so you can use the open {{ quote_media }} field. We can always decide whether we want the nested media.twig later.

Thanks, It doesn't appear I will be getting the cycles to get to that work so merging this as is because it doesn't hurt.

@sherakama sherakama merged commit 7fbc39f into master May 27, 2020
@sherakama sherakama deleted the D8CORE-1632 branch May 27, 2020 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yvonnetangsu yvonnetangsu yvonnetangsu approved these changes

@jenbreese jenbreese Awaiting requested review from jenbreese

Assignees

@jenbreese jenbreese

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@sherakama @jenbreese @yvonnetangsu