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

fixes #1643, not display fb,tw,email buttons if not configured #2008

Closed
wants to merge 1 commit into from

Conversation

techac
Copy link
Contributor

@techac techac commented Oct 23, 2018

Closes #1643

[ ] Generates new fake data, if necessary
[ ] Wagtail documentation has been updated, if necessary
[ ] Includes tests, if appropriate
[ ] Migrations squashed

@techac
Copy link
Contributor Author

techac commented Oct 24, 2018

@alanmoo you may review the PR

@alanmoo
Copy link
Contributor

alanmoo commented Oct 24, 2018

Thanks- I'll try to get to it today, though we're trying to tie up the Buyer's Guide so our petition work is a little bit of a lower priority this week.

@techac
Copy link
Contributor Author

techac commented Oct 24, 2018

@alanmoo thanks

@techac
Copy link
Contributor Author

techac commented Oct 29, 2018

@alanmoo can you please update me with my PR

Copy link
Contributor

@alanmoo alanmoo left a comment

Choose a reason for hiding this comment

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

Thanks for your patience, Mozfest was this past weekend so we were super busy! See inline comment below.

</svg>
EMAIL
</button>
{this.props.shareLink ?
Copy link
Contributor

Choose a reason for hiding this comment

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

This code will never execute. The block on line 435 comes after a check for the same variable, so if this.props.shareLink exists, line 435 will be returned and skip the remainder of this function.

There should be multiple checks for variables to resolve this issue- one for facebook, one for twitter, and one for email. The variable you're looking for is the ones defined here. You're going to have to inspect the code to figure out how they eventually get passed to the client.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@alanmoo I got it.. and when I inspected the code there was no prop for sharelink_twiiter, facebook and email ... can you plz help me how can I get these fields from models to jsx as I am new to react... thanks ...

Copy link
Contributor

Choose a reason for hiding this comment

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

Unfortunately if I had the time to trace that, I would be solving this ticket myself. If you can't figure it out, I'd suggest checking out our good first issues

@Pomax
Copy link
Contributor

Pomax commented Dec 11, 2018

I'm going to close this for now, as it's been unresolved for a few months - once this issue makes its way back into the project board, we can revisit it.

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