-
Notifications
You must be signed in to change notification settings - Fork 9
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
✨ feat: Add 'Improve Content' section to summary #159
✨ feat: Add 'Improve Content' section to summary #159
Conversation
Amazing work, @sushma1031 |
@tusharnankani Will do! |
@tusharnankani I edited the pull request description to include the screen recording, let me know if everything's fine! |
Looks perfect! +_+ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm glad I could contribute! Thanks @tusharnankani & @HarshKapadia2 for guiding me through it :) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much for the discussions and PR, @sushma1031!
These are some of the changes that I can think of. You can just directly commit most of them, unless you have further questions on any of them. Do let us know!
I'll work on the changes @HarshKapadia2 , thanks for letting me know |
Refine body content and apply styling to link of second improve button
Modify "View source on Github" link
Modify "View the source on Github" to link content of the sessions
Use 'em' in improve content section styles
@HarshKapadia2 Done with the changes 👍 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me!
Thank you so much for contributing, @sushma1031! Thank you for being prompt as well!
We'll just wait for @KartikSoneji to review the PR as well, as he wanted a particular change with converting classes to IDs, so that we're able to link to those sections using hash fragments. |
Also just a note @sushma1031, that you can directly commit review suggestions by clicking the 'Commit suggestion' button below them. You don't need to manually commit them. (Note that the person who suggests does get co-authorship.) |
My pleasure!
All right.
Ah, all right, I did notice that feature but I wasn't sure if I should use it or not, thanks for letting me know! |
summary/combined-summary.adoc
Outdated
|
||
[.improve-buttons] | ||
* link:https://github.com/OurTechCommunity/catchup/tree/main/summary/sessions[Edit these summaries on GitHub.^] | ||
* link:https://github.com/OurTechCommunity/catchup/issues/new?title=Improve%20Combined%20Summary&body=%3E%20Please%20describe%20the%20issue%20with%20the%20summary%20content%20and%20the%20changes%20to%20be%20made.[Report the content issue.^] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
* link:https://github.com/OurTechCommunity/catchup/issues/new?title=Improve%20Combined%20Summary&body=%3E%20Please%20describe%20the%20issue%20with%20the%20summary%20content%20and%20the%20changes%20to%20be%20made.[Report the content issue.^] |
Do we need to worry about content issues?
Content issues (outdated, deprecated, incorrect information) can appear on docs pages, but I don't think they apply here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh alright. I mean Content in this context could literally mean, the content of the summary.
We can rename that Issue. So, they can report any issue; if they do not want to edit the summaries directly and just point out any issue.
What do you think? I think it is a valid Action
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm ok, that would make more sense.
summary/individual-summary.adoc
Outdated
[.improve-heading] | ||
Want to improve the content of this page? | ||
|
||
[.improve-buttons] | ||
* link:https://github.com/OurTechCommunity/catchup/tree/main/summary/sessions/{catchup_display_number}[Edit this summary on GitHub.^] | ||
* link:https://github.com/OurTechCommunity/catchup/issues/new?title=Improve%20Summary%20No.%20{catchup_display_number}&body=%3E%20Please%20describe%20the%20issue%20with%20the%20summary%20content%20and%20the%20changes%20to%20be%20made.[Report the content issue.^] | ||
* link:https://github.com/OurTechCommunity/catchup/blob/main/summary/sessions/{catchup_display_number}[View the source on GitHub.^] | ||
|
||
|
||
[.contribute] | ||
Want to get more involved? link:https://github.com/OurTechCommunity/catchup/blob/main/CONTRIBUTING.md[Learn how to contribute.^] | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same changes as above.
All right, I'll make the requested changes. Just to clarify, these are the changes to be made:
If I've gotten anything wrong, please let me know. |
Co-authored-by: Kartik Soneji <[email protected]>
Move TOC to left sidebar Add improve content heading
@KartikSoneji Made the requested changes. It now appears like this: |
Looks good. @KartikSoneji, please let us know if we are good to go. |
Thank you so much, @sushma1031 and sorry for the delay in merging the PR! |
I'm glad I could help, and thanks for all the guidance! 😊 |
Fixes issue #146
Demo:
https://github.com/OurTechCommunity/catchup/assets/123486589/7ac7398c-a5eb-40ce-ab04-2f4aa1f6a318