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

Link Contribute #325

Merged
merged 3 commits into from
Jun 25, 2020
Merged

Link Contribute #325

merged 3 commits into from
Jun 25, 2020

Conversation

HarikumarG
Copy link
Contributor

@mansona
Thanks ,
I somewhat understood what you are expecting ..I tried the code which you mentioned...
<EsFooter @contributeLink="https://github.com/ember-learn/ember-website" /> adding this in the files which you said..but i face some error that footer itself is not displaying ..i couldn't get the flow what you are expecting..So I saw those links which are passed as an parameter to <EsFooterStatement/> and done the same think for contribution link also..
If <EsFooter/> tag is used, then again another footer will be added to the existing footer...
If this is not what you are expecting..then let me know...anyways i am about to go through the learn docs once again and i will get it..
Thanks for the guidance..

Copy link
Member

@mansona mansona 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 sticking with this 🎉 I think we're almost there, I think the last thing that we need to change is the fact that it's not supposed to have a default value 👍 once we get that solved from the functional perspective we can discuss the design of it 🎉

addon/components/es-footer.js Outdated Show resolved Hide resolved
addon/constants/es-footer.js Outdated Show resolved Hide resolved
addon/templates/components/es-footer-statement.hbs Outdated Show resolved Hide resolved
addon/templates/components/es-footer.hbs Outdated Show resolved Hide resolved
Copy link
Contributor Author

@HarikumarG HarikumarG left a comment

Choose a reason for hiding this comment

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

@mansona
Again Thanks,
Understood your idea that you don't want to be default link that should exist in all the pages which uses <EsFooter> ...You want this link to be added to ember website alone...I removed all those things which you mentioned as not needed..

Copy link
Member

@mansona mansona left a comment

Choose a reason for hiding this comment

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

Almost there! I don't think I explained myself correctly last time. This should only work if the contribution link is being passed into <EsFooter />. I have made suggestions that should implement the functionality that I'm talking about 👍

Can you also add an example of this working in the documentation, please? The file to change should be /docs/components/footer.md

addon/templates/components/es-footer-statement.hbs Outdated Show resolved Hide resolved
addon/templates/components/es-footer.hbs Outdated Show resolved Hide resolved
addon/components/es-footer.js Outdated Show resolved Hide resolved
addon/components/es-footer.js Outdated Show resolved Hide resolved
@HarikumarG
Copy link
Contributor Author

Almost there! I don't think I explained myself correctly last time. This should only work if the contribution link is being passed into <EsFooter />. I have made suggestions that should implement the functionality that I'm talking about

Can you also add an example of this working in the documentation, please? The file to change should be /docs/components/footer.md

@mansona
Thanks,
I have done the changes which you asked ...Let me know after you checked this PR...If it is fine then i can change this footer.md file...What is that required in that document ? Can you please explain me that ? <EsFooter @contributeLink="https://github.com/ember-learn/ember-website" /> is this is enough in that document ?

@mansona
Copy link
Member

mansona commented May 8, 2020

@HarikumarG yes that's correct. that is all you would need to document, and maybe put a bit of text saying "you can add a link to contribute to this page with the following example: " or something like that 👍

@HarikumarG
Copy link
Contributor Author

HarikumarG commented May 9, 2020

@mansona
Thanks for the guidance..I have updated the footer.md file ...
Now after merging this ...In that ember website we need to pass that URL in that <EsFooter> tag to display that URL ...am i right ?

@mansona mansona mentioned this pull request May 24, 2020
@amyrlam amyrlam self-assigned this Jun 25, 2020
@mansona mansona merged commit 2d45566 into ember-learn:master Jun 25, 2020
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