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

feat: A button to edit/make the summary better via PRs.. #146

Closed
dheerajdlalwani opened this issue Mar 22, 2023 · 29 comments · Fixed by #159
Closed

feat: A button to edit/make the summary better via PRs.. #146

dheerajdlalwani opened this issue Mar 22, 2023 · 29 comments · Fixed by #159
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers summary Related to OTC CatchUp session summaries

Comments

@dheerajdlalwani
Copy link
Member

image

The button can be added here^

Or......
Like NextJS documentation adds it in the footer

image

@HarshKapadia2
Copy link
Member

Good idea, @dheerajdlalwani!

Some options for the button text:

  • Suggest changes
  • Edit summary on GitHub

@HarshKapadia2 HarshKapadia2 added enhancement New feature or request good first issue Good for newcomers summary Related to OTC CatchUp session summaries labels Mar 25, 2023
@sushma1031
Copy link
Contributor

I'd like to work on this.

I'd like to request some clarification:

  1. Are there any specifications about the appearance of the button?
    1.1. In the Next.js example, it's just displayed as a link, is the same required here as well?
  2. Should the button direct them to the GitHub repo, or something more specific?

@HarshKapadia2
Copy link
Member

HarshKapadia2 commented May 14, 2023

I'd like to work on this.

Awesome! Thank you!

I'd like to request some clarification:

  1. Are there any specifications about the appearance of the button?
    1.1. In the Next.js example, it's just displayed as a link, is the same required here as well?

A link at the top of the page would be just fine!

  1. Should the button direct them to the GitHub repo, or something more specific?

Do let us know if you have any more questions! Also, do add pictures of how the pages look like, whenever you contribute!

Thank you!

@sushma1031
Copy link
Contributor

I'm having trouble adding a link at the top of the page. This is my code for a test link:

= OTC CatchUp #{catchup_display_number} Summary
Our Tech Community
v1
:revremark: link:https://catchup.ourtech.community[Test Link^]

This is how it renders:
Screenshot 2023-05-14 200943

Could you please suggest the appropriate syntax?

Alternatively, I could add the link at the bottom:
image

@HarshKapadia2
Copy link
Member

HarshKapadia2 commented May 14, 2023

I think you're trying to add the link at the wrong place.

Try adding link:https://github.com/OurTechCommunity/catchup/tree/main/summary/sessions/130[Edit this summary on GitHub^] above line no. 17 of the individual-summary.adoc file:

@sushma1031
Copy link
Contributor

I gave that a try, this is the result:
image

If this is the required result, please let me know, I'll do the same for the combined summary as well

@HarshKapadia2
Copy link
Member

Should we reduce the font size of the 'edit' link, @tusharnankani?

@HarshKapadia2
Copy link
Member

@sushma1031 can you please try reducing the font size and send a screenshot of that?

Feel free to send pictures with any other looks that you think work better as well.

Cc: @tusharnankani

@sushma1031
Copy link
Contributor

Will do!

@sushma1031
Copy link
Contributor

Reducing the font size to 1rem appears like this:
1rem

Code:

[.edit-summary-btn]
link:https://github.com/OurTechCommunity/catchup/tree/main/summary/sessions/{catchup_display_number}[Edit this summary on GitHub^]

CSS

.edit-summary-btn a{
	font-size: 1rem;
}

@sushma1031
Copy link
Contributor

I personally found the extra space at the top unappealing:
1rem spacing

Hence I tried to make it even using css relative position
1rem with relative position

Let me know your thoughts

@sushma1031
Copy link
Contributor

sushma1031 commented May 26, 2023

Applying 0.75 rem made it look too small according to me, however let me know your opinion:
point75rem

@tusharnankani
Copy link
Member

Hey

Looks good, yeah.

Should we reduce the font size of the 'edit' link, @tusharnankani?

Yes.

Also, what do you think if we push the button to the bottom?

image

Something like this at MDN: https://developer.mozilla.org/en-US/docs/Web/javascript

@sushma1031
Copy link
Contributor

sushma1031 commented May 27, 2023

Sure @tusharnankani , I can try that. It would look similar to this, although here I haven't reduced the font size yet:

Alternatively, I could add the link at the bottom: image

Also, should I add a message above the link, like in the MDN docs, or is just the link sufficient?

cc: @HarshKapadia2

@tusharnankani
Copy link
Member

I definitely think it is a good idea to add enough context like MDN.

What do you think @OurTechCommunity/core?

@HarshKapadia2
Copy link
Member

Yeah I think adding a section like MDN at the bottom is a nice idea!

@sushma1031 can you design something like MDN that you like? We can go ahead from there. Do post a screenshot!

@tusharnankani
Copy link
Member

Let us know if you need any help!

@sushma1031
Copy link
Contributor

I'll work on it.
I'd like to request some more information, such as what we want to include in this section, in addition to the edit summary link.

@tusharnankani
Copy link
Member

Sure.

Here is a layout you could follow:

Improve the content on this page?

[Edit the summary on GitHub](https://github.com/OurTechCommunity/catchup/blob/main/summary/sessions/130/content.adoc).
[Report the content issue](https://github.com/OurTechCommunity/catchup/issues/new?title=Improve%20%3Cnum%3E%20summary&body=Enter%20summary%20URL%20and%20changes%20to%20be%20made:).
[View the source on GitHub](https://github.com/OurTechCommunity/catchup/blob/main/summary/sessions/130/content.adoc).

Want to get more involved? [Learn how to contribute](https://github.com/OurTechCommunity/catchup/blob/main/CONTRIBUTING.md).

Renders like:

Improve the content on this page?

Edit the summary on GitHub.
Report the content issue.
View the source on GitHub.

Want to get more involved? Learn how to contribute.

@HarshKapadia2
Copy link
Member

Do modify the links to include the correct CatchUp numbers, @sushma1031 !

Thank you for the example, Tushar!

@sushma1031
Copy link
Contributor

I'll make sure to include the respective numbers @HarshKapadia2.
Thank you for the layout @tusharnankani !

@sushma1031
Copy link
Contributor

Here's how it looks now:
image

Here's my code:

[.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%20%3C{catchup_display_number}%3E%20summary&body=Enter%20summary%20URL%20and%20changes%20to%20be%20made[Report the content issue.^]
* link:https://github.com/OurTechCommunity/catchup/blob/main/summary/sessions/{catchup_display_number}/content.adoc[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.^]

Let me know if any changes are required.

Also, my CSS changes are being ignored by Git, as the summary-style.css is present in the .gitignore file. Could you please let me know what I should do?

@HarshKapadia2
Copy link
Member

Here's how it looks now: image

I like how this looks! What do y'all think @OurTechCommunity/core?

I think we need to debate on the position of this section. It looks good at the bottom, but I think it should be above the quotes and below the note with the blue icon. (Picture below.) (We might get rid of that note, but that's another issue - we really need to restructure our summary page.)



Also, my CSS changes are being ignored by Git, as the summary-style.css is present in the .gitignore file. Could you please let me know what I should do?

Summary styles are located in https://github.com/OurTechCommunity/catchup/blob/main/summary/static/css/summary-style.css, so please make your changes in that file. (I think the file that you're referring to is the file that is copied over to public/css/summary while building the site.)

@sushma1031
Copy link
Contributor

Summary styles are located in https://github.com/OurTechCommunity/catchup/blob/main/summary/static/css/summary-style.css, so please make your changes in that file. (I think the file that you're referring to is the file that is copied over to public/css/summary while building the site.)

I was trying to update the wrong file as you pointed out. Thanks for directing me to the right one!

@tusharnankani
Copy link
Member

One second, just realized this is an issue.

@sushma1031, please feel free to open a PR.
The UI looks great :D

@tusharnankani
Copy link
Member

tusharnankani commented May 29, 2023

I think we need to debate on the position of this section. It looks good at the bottom, but I think it should be above the quotes and below the note with the blue icon.

Yes, let's place the content above the horizontal rule above the quotes. Would look good hopefully!

We might get rid of that note, but that's another issue - we really need to restructure our summary page.

Right. Let's figure that out at #113

@sushma1031
Copy link
Contributor

This is how it looks:
image
I was wondering if I should change the colour of "Want to improve the content..." to match these sub-headings:
Screenshot (112)

@sushma1031
Copy link
Contributor

sushma1031 commented May 30, 2023

Following the same layout for the combined summary page:
Screenshot (114)

Code:

[.improve-heading]
Want to improve the content of this page?

[.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=Enter%20changes%20to%20be%20made[Report the content issue.^]
* link:https://github.com/OurTechCommunity/catchup/blob/main/summary/combined-summary-template.adoc[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.^]

@HarshKapadia2
Copy link
Member

I was wondering if I should change the colour of "Want to improve the content..." to match these sub-headings:
Screenshot (112)

No, let's keep the normal black text for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers summary Related to OTC CatchUp session summaries
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants