OurTechCommunity / catchup

The OTC CatchUp web app and workflows.
https://catchup.ourtech.community
MIT License
15 stars 9 forks source link

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

Closed dheerajdlalwani closed 1 year ago

dheerajdlalwani commented 1 year ago

image

The button can be added here^

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

image

HarshKapadia2 commented 1 year ago

Good idea, @dheerajdlalwani!

Some options for the button text:

sushma1031 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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:

https://github.com/OurTechCommunity/catchup/blob/72531547ec028e38f64c97c0621e5b8c5ccad2be/summary/individual-summary.adoc?plain=1#L17

sushma1031 commented 1 year ago

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 commented 1 year ago

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

HarshKapadia2 commented 1 year ago

@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 commented 1 year ago

Will do!

sushma1031 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

point75rem
tusharnankani commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

What do you think @OurTechCommunity/core?

HarshKapadia2 commented 1 year ago

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 commented 1 year ago

Let us know if you need any help!

sushma1031 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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

Thank you for the example, Tushar!

sushma1031 commented 1 year ago

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

sushma1031 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

One second, just realized this is an issue.

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

tusharnankani commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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.