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

Header donation #91

Merged
merged 8 commits into from
Nov 20, 2020
Merged

Header donation #91

merged 8 commits into from
Nov 20, 2020

Conversation

duranmla
Copy link
Contributor

@duranmla duranmla commented Nov 19, 2020

What:
Add support to inject custom donation component

Why:
relates to the effort of
Gatsby link for Donate button

How:
Use the slot API from stencil to allow render a custom component whenever the donateurl is set to falsy value
The need of use a donateurl to false comes from the bug that has been addressed with ionic-team/stencil#2650

Extras:
This code will add donate button on mobile and desktop for users and guests, besides some other aesthetics details.

Media:

@duranmla duranmla requested review from orlando and arathjz November 19, 2020 19:02
Copy link
Contributor

@orlando orlando left a comment

Choose a reason for hiding this comment

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

I left a comment related to the workaround we are doing

<dc-user-items user={this.user} community={this.community} />
<div class="with-user">
<slot name="donate">
{this.donateurl && (
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we remove this now the fix has landed on Stencil?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It has been merge to master on the stenciljs project but it hasn't get released yet @orlando
https://github.com/ionic-team/stencil/commits/master currently the latest version is 2.3.0 and it doesn't have the fix yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have created a technical debt for this one so we can move forward now

@duranmla duranmla merged commit a9d0794 into master Nov 20, 2020
@duranmla duranmla deleted the header-custom-session-links branch November 20, 2020 16:18
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