website icon indicating copy to clipboard operation
website copied to clipboard

Guides Pages Redesign

Open abenipa3 opened this issue 2 years ago • 12 comments

Fixes #1630

What changes did you make and why did you make them?

This pull request contains files to the Guide Pages Redesign.

The following files have been added to the respective folders:

  • _guide-pages\how-to-set-reminders-in-slack.md - New Guide Page called "How to Set Reminders in Slack"
  • _includes\toc.html - Generates section and subsection titles for the sticky navigation of the Guide Pages.
    • This is the same method used for 100Automations' Guide Pages as seen [here].
  • _layouts\guides.html - Layout used for the Guides' MD files.
  • _sass\components\_guides.scss - Styling for the redesigned Guide Pages.
  • assets\images\guides\how-to-set-reminders-in-slack - Contains all of the images included in "How to Set Reminders in Slack"
  • assets\images\guides\guide-page-hero.svg - New hero image for the redesigned Guide Pages.
  • assets\js\guidepages.js - JS for Guide Pages that enables the functionality of the sticky navigation bar and the Return to the Top Button.
    • Return to the Top Button is only visible on mobile.
  • pages\guides.html

The following file has been modified:

  • _sass\main.scss - Added @import 'components/guides.scss'; to import styling for the redesigned Guide Pages.

Screenshots of Proposed Changes Of The Website (if any, please do not screen shot code changes)

Visuals before changes are applied

Current Guide Page Design

image

Visuals after changes are applied

Redesigned Guide Page Design

Note: Please excuse the odd navbar placements. I used an extension to snapshot the entire webpage and the navbar followed the extension as it scanned the page.

image

Here is also the Figma Link to the Guide Pages Redesign as reference.

Special Notes

~~The following issues need to be completed before the Redesigned Guide Page is ready for public view: #2978 - Create Guides Markdown Converter (Google Apps Script)~~

Update 6/4/22: After a discussion with our team, PR will be merged into gh-pages after review.

abenipa3 avatar May 20 '22 03:05 abenipa3

Want to review this pull request? Take a look at this documentation for a step by step guide!

From your project repository, check out a new branch and test the changes.

git checkout -b alyssabenipayo-guides-redesign-1630 gh-pages
git pull https://github.com/alyssabenipayo/website.git guides-redesign-1630

github-actions[bot] avatar May 20 '22 03:05 github-actions[bot]

Converting this PR to draft as it is not ready to be reviewed yet. Also, planning to create a new feature branch so that the conditions in special notes above could be satisfied.

SAUMILDHANKAR avatar May 23 '22 21:05 SAUMILDHANKAR

Availability: 3 hours Review ETA: Thursday, June 2

tamara-snyder avatar May 29 '22 17:05 tamara-snyder

Based on further discussion during the team meetings, this PR is ready for review. We are just going to merge this to gh-pages branch. Factors considered for this decision: it takes a lot of time to manage feature branches. Also, since the link for this page isn't shared on our hfla website yet, so the page does not have a lot of visibility and it should be fine even though #2978 isn't done.

SAUMILDHANKAR avatar May 29 '22 17:05 SAUMILDHANKAR

Avail: 2hrs ETA: 6/2/22 EOD

Sparky-code avatar Jun 01 '22 01:06 Sparky-code

@alyssabenipayo

Ongoing Review (Revised ETA) Sorry, it was a bit of a busier week than expected. Just getting a chance to look at the issue now. Will probably be a slower review as I have a few questions before I start.

  • I believe this issue is just looking at the implementation of the design layout (in anticipation of the markdown converter making this process automated)? More?

  • At first glance it looks like you're adding the right things and its building properly but I'm a bit lost as to what the scope of this review/issue is with all the moving parts on the linked issue. Thanks!

Else:

  • Are the action items in the beginning of the linked issue relevant or should I just pay attention to the action items listed in these comments here ? Other comments as well?

  • Is this issue looking at any of the liquid issues or workflow buildouts mentioned in the linked comments?

Sparky-code avatar Jun 04 '22 16:06 Sparky-code

@sparky-code

Thanks for reviewing my PR.

  • I believe this issue is just looking at the implementation of the design layout (in anticipation of the markdown converter making this process automated)? More?
  • At first glance it looks like you're adding the right things and its building properly but I'm a bit lost as to what the scope of this review/issue is with all the moving parts on the linked issue. Thanks!
  • Yes, the scope of this review is to look at the implementation of the design layout. Thank you for clarifying, by the way. I just updated the PR notes by deleting the special notes since this Redesigned Guide Page will be merged to gh-pages branch, and this page is not shared on our HFLA website yet.

Else:

  • Are the action items in the beginning of the linked issue relevant or should I just pay attention to the action items listed in these comments here ? Other comments as well?
  • Great questions! The action item relevant to this PR is defining the MVP. In this case, the redesigned guide pages is our MVP, and we want to review the styling/build of the page.
    • Side note: This issue contains both comments related to the design and development of the Guide Pages; the majority of the comments up to 10/28/22 are design-related (and testing markdown converter, which will be further covered in another issue - https://github.com/hackforla/website/issues/2978). The rest of the comments are development-related. Most importantly, we want to review the style and the build of the Guide Page.

Is this issue looking at any of the https://github.com/hackforla/website/issues/1630#issuecomment-899001930 or https://github.com/hackforla/website/issues/1630#issuecomment-848223610 mentioned in the linked comments?

  • If the liquid involves the previously mentioned, then no. We only want to focus on the styling aspect of the redesigned Guide Page in this PR + issue.
  • Both the liquid issue (not related to styling) and workflow buildouts will be covered in https://github.com/hackforla/website/issues/2978.

Thank you again for reviewing my PR. Please feel free to reach out to me if you have further questions!

abenipa3 avatar Jun 04 '22 23:06 abenipa3

Sorry for the delay, its been a busy week - ETA: Tuesday 6/14 EOD Avail: 1-3hrs Blockers: None, Questions answered

Sparky-code avatar Jun 12 '22 17:06 Sparky-code

Revised ETA: Thursday EOD Blockers: Caught a bad cold and having some issues getting the page to load locally

Sparky-code avatar Jun 15 '22 20:06 Sparky-code

Review ETA --> Hi @alyssabenipayo , sorry for the late update. It is a pretty large issue, @tamara-snyder and I will get together at some point to review together and also work individually to review it.

Hopefully have it reviewed by the end of this week -> Sunday (July 25th, 22) EOD.

usaboo avatar Jul 17 '22 17:07 usaboo

Hi @usaboo, all good! Thanks for letting me know! Please feel free to let me know if you have any questions. I'm still available to make any changes if needed. 👍

abenipa3 avatar Jul 17 '22 18:07 abenipa3

@Wny-Duong Thanks so much for your feedback! I'll be picking up this PR with proper responses either tonight or next Monday as I'll be on vacation for the rest of the week.

ETA: Friday, 8/19 the latest

abenipa3 avatar Aug 11 '22 14:08 abenipa3

@Wny-Duong Thank you so much again! I greatly appreciate your time in reviewing this large PR and the details in your comments. 😄 Please see below for my responses.

  1. Clicking on the links on the right side seem to redirect you to the line immediately below the label as opposed to the line corresponding with the link text itself. I'm not too sure if this is intended behavior or not. Example: Clicking on What is Slack? gets you slightly below the header for What is Slack?

Great catch! (I'm going to circle back to this later and I investigate. Will update this as soon as it's resolved.)

  1. From the original Figma, it looks the sections: Who uses reminders and Type of Reminders (line 44 and line 49 in _guide-pages/how-to-set-reminders-in-slack.md should be using the the same header CSS as What is Slack? and When to use Reminders?.

No changes were applied. The font sizes for Who Uses Reminders and Type of Reminders differ from What is Slack? and When to Use Reminders? since they are H2.

See Here for Font Details

What is Slack? - H1, Font Size: 36px image

When to Use Reminders? - H1, Font Size: 36px image

Who Uses Reminders - H2, Font Size: 24px image

Type of Reminders - H2, Font Size: 24px image

  1. In When to use Reminders (line 51 of _guide-pages/how-to-set-reminders-in-slack.md) , there's some strange text that goes: **allows you to set up an automatic reminder with a tailored message ** It doesn't look like this text is meant to be bolded according to the Figma page.

Done ✅ That was an oversight on my part when I created the Figma design. The text was supposed to be bolded to match the original document in our Google Docs. I've applied the changes by fixing the bolded text in the line 51 of _guide-pages/how-to-set-reminders-in-slack.md file and the Figma Design. Thank you!

image

  1. There's a numbering issue on the page under option 1 (starting below line 73 to line 99 of _guide-pages/how-to-set-reminders-in-slack.md), as the steps all start with 1. instead of being properly numbered from 1-4.

Done ✅ Added a line break to add a space between the steps and the images, specifically for steps 1, 2, and 4, so that Jekyll can recognize that the steps should be properly numbered from 1-4.

Applied Changes - Image Preview

image

image

image

  1. A similar issue is seen in Option 2: by Direct Entry (starting below line 114 to line 123 of _guide-pages/how-to-set-reminders-in-slack.md) where the numbering resets on the step starting with "To Whom"

Done ✅ Applied the changes similar to Option 1.

Applied Changes - Image Preview

image

Also on the second step (line 119 to 121 of _guide-pages/how-to-set-reminders-in-slack.md) it looks like there should've been subpoints or some other special formatting for the reminder recipients, "specific person" and "entire channel" since they both appear on the same line instead of separated. The differences are shown below between what's on the current page and the figma design.

Done ✅ I've also updated the font color to the Figma since apparently I didn't add the color the "specific person" and "entire channel" as it appeared on the Google Docs. Applied changes can be seen in the previous comment.

Side note/Commentary: I think ideally (at the time) we wanted to avoid applying CSS (and HTML) to the MD files, but applying CSS was how I could replicate what we had in our design...at least until we find out how to add that to our Automated MD Converter. Other than that, I'm open to suggestions on how to best apply the special formatting to this section. 🙌

Side-Side Note: Not done updating this PR. Committing changes in the meantime till I come back tomorrow or Friday. Will update you all once it's resolved. Thanks!

abenipa3 avatar Aug 18 '22 04:08 abenipa3

~~I was trying to get the latest updates while committing my changes, but didn't work out so I deleted my forked repo in an attempt at a fresh start. I realized was a bad idea as it closed this PR. Resolving this atm~~ my apologies.

Update 8/18/22: Issue resolved. These links helped. The only thing is that I need to create a new PR...@Wny-Duong

  • https://stackoverflow.com/questions/20977530/recover-a-commit-sent-as-a-pull-request-from-a-deleted-fork-on-github
  • https://stackoverflow.com/questions/54033842/how-to-pull-a-pull-request-from-upstream-in-github

abenipa3 avatar Aug 18 '22 05:08 abenipa3

Closed PR. Recreated here - https://github.com/hackforla/website/pull/3476

abenipa3 avatar Aug 19 '22 05:08 abenipa3