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

CPS-332: Button markup fix in PremiumBlock.tpl #18799

Merged
merged 1 commit into from
Oct 22, 2020
Merged

CPS-332: Button markup fix in PremiumBlock.tpl #18799

merged 1 commit into from
Oct 22, 2020

Conversation

deb1990
Copy link
Contributor

@deb1990 deb1990 commented Oct 19, 2020

Overview

In https://github.com/civicrm/civicrm-core/pull/18410/files#diff-6b993747c85ceec4b537726331c934f4aeac58a54152f850dab5aec2942a1009L65, the button markup was not fixed correctly, so this PR fixes the same.

Before

2020-10-19 at 4 44 PM

After

2020-10-19 at 4 45 PM

@civibot
Copy link

civibot bot commented Oct 19, 2020

(Standard links)

@civibot civibot bot added the master label Oct 19, 2020
@eileenmcnaughton
Copy link
Contributor

@agh1 - can you check this?

@deb1990 - can you switch the PR to being against the 5.31 rc branch? While not exactly a regression this is part of the button changes in that release & a case could made to call it a regression

@@ -62,7 +62,9 @@
<div class="premium-full-title">{$row.name}</div>
<div class="premium-full-disabled">
{ts 1=$row.min_contribution|crmMoney}You must contribute at least %1 to get this item{/ts}<br/>
<button type="button" value="{ts 1=$row.min_contribution|crmMoney}Contribute %1 Instead{/ts}" amount="{$row.min_contribution}" />
<button class="btn btn-secondary" type="button" amount="{$row.min_contribution}">
Copy link
Contributor

Choose a reason for hiding this comment

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

I wasn't aware we were putting in bootstrap classes here? should we be using crm-button class instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. I removed the classes and it still works.

@deb1990 deb1990 changed the base branch from master to 5.31 October 20, 2020 05:31
@civibot civibot bot added 5.31 and removed master labels Oct 20, 2020
@eileenmcnaughton
Copy link
Contributor

@deb1990 is this still WIP per the title?

@deb1990 deb1990 changed the title [WIP] CPS-332: Button markup fix in PremiumBlock.tpl CPS-332: Button markup fix in PremiumBlock.tpl Oct 21, 2020
@deb1990
Copy link
Contributor Author

deb1990 commented Oct 21, 2020

@eileenmcnaughton I have moved it out of WIP now.
@seamuslee001 Fixed the issue mentioned by you.

@eileenmcnaughton
Copy link
Contributor

I'm going to merge this - I think waiting for @agh1 to weigh in is less important than getting known button issues fixed in the rc to make it solid for testing

@eileenmcnaughton eileenmcnaughton merged commit 397e8a9 into civicrm:5.31 Oct 22, 2020
@deb1990 deb1990 deleted the CPS-332-support-new-button-markup branch October 23, 2020 10:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants