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

Guides Pages Redesign - Updated (Retry) #3548

Merged
merged 10 commits into from
Sep 13, 2022

Conversation

abenipa3
Copy link
Member

@abenipa3 abenipa3 commented Sep 13, 2022

Fixes #1630

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

(Please see Update 9/13/22 below)

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.

Link to View Locally: http://localhost:4000/guide-pages/how-to-set-reminders-in-slack.html

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.

Update 8/18/22:
I made the mistake of deleting my head/forked repo. My intent was to pull the latest updates as I was committing my changes to my old PR - #3161 -, but had issues pulling the latest updates, so deleted my fork repo (bad idea).

Was able to retrieve the old PR, but could no longer update from there; Github needed me to create a new one. My apologies for the inconvenience once again. Please feel free to message me if you have any questions via Slack or email ([email protected]). Thank you.

Update 9/13/22:
Reopened - we detected visual issues found after merging PR. [Link to Initial PR]

Links to Related Issues

Some existing issues in this PR may have already been addressed as action items in related issues. If an issue has not been addressed, please feel free to mention it here or in the related issues. Here are the links as references:
#1515 - Create mobile version of the Guide Pages and ensure responsiveness
#3170 - Guide Pages - Sticky Navigation Fix
#2978 - Create Guides Markdown Converter (Google Apps Script)
#1630 (comment) - Guide Pages Design System Update

@github-actions
Copy link

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

@abenipa3 abenipa3 added P-Feature: Toolkit https://www.hackforla.org/toolkit/ role: design labels Sep 13, 2022
@github-actions github-actions bot added role: front end Tasks for front end developers size: 13+pt Must be broken down into smaller issues Complexity: Large Status: Updated No blockers and update is ready for review labels Sep 13, 2022
@abenipa3 abenipa3 removed Complexity: Large Status: Updated No blockers and update is ready for review size: 13+pt Must be broken down into smaller issues labels Sep 13, 2022
@github-actions github-actions bot added size: 13+pt Must be broken down into smaller issues Complexity: Large Status: Updated No blockers and update is ready for review labels Sep 13, 2022
@abenipa3 abenipa3 added Status: Updated No blockers and update is ready for review and removed Status: Updated No blockers and update is ready for review labels Sep 13, 2022
@abenipa3
Copy link
Member Author

abenipa3 commented Sep 13, 2022

Hey team! I may need assistance with this. For reference, see below. [Link]

Thank for getting back to me Alyssa. Please forgive me, I misspoke earlier, when I said "re-open" I meant create a PR. We tried to "re-open" the PR, obviously to no avail.

Below are screenshots I grabbed of the live website when we (foolishly) merged the PR.

Screenshot (241)

Screenshot (240)

Here is the output after I checked locally:
image

image

image

I suspect it might be a thing with GH Pages where new changes may take up to 10 minutes if not an hour. (I also realized I've encountered a similar issue in the past and had to wait for about an hour for all the contents to populate properly.) - Here's a doc for reference, too - https://docs.github.com/en/pages/getting-started-with-github-pages/creating-a-github-pages-site.

If anything, I'm open to options on how we can approach this. Would very much appreciate it if anyone could double-check again locally just in case. 😄 👍

@blulady
Copy link
Member

blulady commented Sep 13, 2022

It's entirely possible. This is what that page currently looks like. I am happy to throw it up locally and if it looks good merge it again. We will meet tonight and I'll double check it. If not roll it back again.
Screenshot (242)

@blulady blulady self-requested a review September 13, 2022 22:23
@abenipa3
Copy link
Member Author

Hey @blulady ! Awesome, sounds a great plan. Thanks so much. I'll look forward to any updates 👍

@ericvennemeyer ericvennemeyer self-requested a review September 13, 2022 22:33
@ericvennemeyer
Copy link
Member

Hey @alyssabenipayo @blulady I just tested this out locally, and it looks fine on my end. FWIW it looks like the "_guides.scss" file is not properly connecting after the merge. That seems to be the one that contains all the style info for the cards, margins, sticky nav, etc. I have no idea why this would work locally but not in production, though. :/

@blulady
Copy link
Member

blulady commented Sep 13, 2022

Hey @alyssabenipayo @blulady I just tested this out locally, and it looks fine on my end. FWIW it looks like the "_guides.scss" file is not properly connecting after the merge. That seems to be the one that contains all the style info for the cards, margins, sticky nav, etc. I have no idea why this would work locally but not in production, though. :/

@ericvennemeyer That's interesting. How can you tell it's not connecting properly?

@blulady
Copy link
Member

blulady commented Sep 13, 2022

Hey, so I am uncertain if this is working as intended or if it just looks a little funky because I am inspecting it... But it seems like the Nav Bar is very centered in these mobile views.
Screenshot (243)
Screenshot (245)
Screenshot (246)

@ericvennemeyer
Copy link
Member

@blulady I'm not 100% it's the _guides.scss file, but at a glance it looks like the styles that aren't being applied are the ones contained in that file.

The sticky nav bar definitely doesn't show up where it's supposed to on mobile views. I mentioned that to @alyssabenipayo as well and apparently it's being covered in a separate issue.

@blulady
Copy link
Member

blulady commented Sep 13, 2022

@blulady I'm not 100% it's the _guides.scss file, but at a glance it looks like the styles that aren't being applied are the ones contained in that file.

The sticky nav bar definitely doesn't show up where it's supposed to on mobile views. I mentioned that to @alyssabenipayo as well and apparently it's being covered in a separate issue.

Ok, if it's getting covered somewhere else, I'll roll it out. I don't suppose you know the issue number?

Copy link
Member

@blulady blulady left a comment

Choose a reason for hiding this comment

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

The sticky nav bar definitely doesn't show up where it's supposed to on mobile views. I mentioned that to @alyssabenipayo as well and apparently it's being covered in a separate issue.

@blulady blulady removed the request for review from ericvennemeyer September 13, 2022 23:19
@abenipa3
Copy link
Member Author

abenipa3 commented Sep 13, 2022

Ok, if it's getting covered somewhere else, I'll roll it out. I don't suppose you know the issue number?

Yes, the issue number is here: #1515 !

Side comment: I wonder if there's a other steps that needed to be covered so that the scss transfers over? It's also added as @import guides.scss (not verbatim 😅 ) in the main.scss. 😮

@blulady blulady merged commit b5fedfa into hackforla:gh-pages Sep 13, 2022
@abenipa3
Copy link
Member Author

abenipa3 commented Sep 13, 2022

Just saw it deployed and here's the output! 😮 👀
image

@ericvennemeyer
Copy link
Member

@alyssabenipayo Yaay! It looked broken to me, but I realized I had to do a hard browser refresh and now it looks good. As you said, it just took a minute for the changes to take effect. 🙌

@blulady
Copy link
Member

blulady commented Sep 13, 2022

It looks good!! Thank you for all of your hard work with this.

@abenipa3
Copy link
Member Author

Wooo-hooo! I'm glad it works! Thanks so much for working with me on this, @ericvennemeyer and @blulady! 😸 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Complexity: Large P-Feature: Toolkit https://www.hackforla.org/toolkit/ role: design role: front end Tasks for front end developers size: 13+pt Must be broken down into smaller issues Status: Updated No blockers and update is ready for review
Projects
Development

Successfully merging this pull request may close these issues.

Work flow for guides to appear on the website
4 participants