Skip to content

Add locations info to about page - #8810

Open
KyleA99 wants to merge 7 commits into
hackforla:gh-pagesfrom
KyleA99:add-locations-info-about-page-8431
Open

KyleA99 wants to merge 7 commits into
hackforla:gh-pagesfrom
KyleA99:add-locations-info-about-page-8431

Conversation

@KyleA99

@KyleA99 KyleA99 commented Sep 28, 2026 •

Copy link
Copy Markdown
Member

Fixes #8431

What changes did you make?

  • I created a new about-card-our-locations.html file in _includes/about-page/

  • I created a new our-locations-images.html file in _includes/about-page/

  • I added the new about-card-our-locations.html file to /pages/about.html

  • I added a new li for the new card to the sticky-nav element

  • Note: Please see the comment I made regarding potential deficiencies in this pull request.

Why did you make the changes (we will use this info to test)?

  • These changes were added to provide information regarding our transition from being an in-person organization to fully-remote.
  • Information was also included to indicate our transition to remote-first collaboration was largely fueled by the COVID-19 pandemic.

CodeQL Alerts

After the PR has been submitted and the resulting GitHub actions/checks have been completed, developers should check the PR for CodeQL alert annotations.

Check the PR's comments. If present on your PR, the CodeQL alert looks similar as shown

Screenshot 2024-10-28 154514

Please let us know that you have checked for CodeQL alerts. Please do not dismiss alerts.

  • I have checked this PR for CodeQL alerts and none were found.
  • I found CodeQL alert(s), and (select one):
    • I have resolved the CodeQL alert(s) as noted
    • I believe the CodeQL alert(s) is a false positive (Merge Team will evaluate)
    • I have followed the Instructions below, but I am still stuck (Merge Team will evaluate)
Instructions for resolving CodeQL alerts

If CodeQL alert/annotations appear, refer to How to Resolve CodeQL alerts.

In general, CodeQL alerts should be resolved prior to PR reviews and merging

Screenshots of Proposed Changes To The Website (if any, please do not include screenshots of code changes)

Visuals before changes are applied Screenshot 2026-09-27 at 11 29 24 PM
Visuals after changes are applied Desktop-Expanded Mobile-Expanded

@github-actions

Copy link
Copy Markdown

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 KyleA99-add-locations-info-about-page-8431 gh-pages
git pull https://github.com/KyleA99/website.git add-locations-info-about-page-8431

@github-actions github-actions Bot added role: front end Tasks for front end developers role: back end/devOps Tasks for back-end developers Complexity: Medium P-Feature: About Us https://www.hackforla.org/about/ P-Feature: Events https://www.hackforla.org/events/ size: 1pt Can be done in 4-6 hours HLC: M Homepage Launch Countdown Must Have labels Sep 28, 2026
@KyleA99

KyleA99 commented Sep 28, 2026

Copy link
Copy Markdown
Member Author
  • The subheading sections “From In-Person…” and “Former in-person...” Are not styled completely correctly as I didn’t see any classes in _about.scss that accurately dealt with font-size and weight for subheaders.

  • I did not see an SVG for the locations card in /assets/images/about/section-header-elements/

  • The image’s corners are defaulting to black - this was not an issue on the events page as the dark background hid this. However, for the about page, light background displays the dark corners.

  • The images are not in an individual row. Rather there is a row for each image. I think this is due to the width of the .page-card—about class not being wide enough to accommodate 3 .event-card items (414px each)

  • There was no class for the horizontal divider line above “Former in-person…” text section.

  • So, as a summary, my feature branch does not line up exactly with the mockup because I wasnt sure if I was allowed to start making custom css classes (even if I followed the H4LA style guide). It sounded like collaboration with designers and more tenured devs is required for this.

@nathanjkim-codes
nathanjkim-codes self-requested a review September 28, 2026 20:33
@nathanjkim-codes

Copy link
Copy Markdown
Member

Review ETA: 9/28 EOD

Availability:
Monday: 8:00 PM - 11:00 PM PT
Tuesday: 8:00 PM - 11:00 PM PT

@egcuriel

Copy link
Copy Markdown
Member

Review ETA: 09/30/2026 EOD
Availability: M-F (8 pm - 11 pm)

@nathanjkim-codes

Copy link
Copy Markdown
Member

Hi @KyleA99, thank you for working on this!

I tested the PR locally and also read your comment about the styling.

I was able to see the same things you mentioned. The three location cards are showing in separate rows, the horizontal line is missing, and the subheadings look different from the mockup.

I understand that you were not sure if you should add new CSS classes for these changes. I think it would be good to ask the team how they want to handle the styling.

I also noticed one small thing. The issue says “Our Locations,” but the mockup says “Our Location,” and the current PR also says “Our Location.” Maybe we should confirm which title we should use.

Thank you!

Comment thread _includes/about-page/about-card-our-locations.html Outdated
@KyleA99
KyleA99 requested a review from egcuriel September 30, 2026 01:59
@KyleA99

KyleA99 commented Sep 30, 2026 •

Copy link
Copy Markdown
Member Author

@egcuriel @nathanjkim-codes agreed with the "Location" -> "Locations" change. I fixed that. I'm assuming this was correct, and if it isnt, merge team will address.

@anthonylo87
anthonylo87 self-requested a review September 30, 2026 02:14
@anthonylo87

Copy link
Copy Markdown
Member

Review ETA: 09/30/2026 EOD
Availability: M-F evenings

@HackforLABot HackforLABot mentioned this pull request Sep 30, 2026
16 of 34 tasks
egcuriel
egcuriel previously approved these changes Sep 30, 2026

@egcuriel egcuriel left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hi @KyleA99,

As you mentioned your comments there some discrepancies between the mockup and the changes requested on the parent issue. You've followed the instructions on the parent issue, so I approve. If there is any new changes please request a review from me. Thank you!

@anthonylo87

Copy link
Copy Markdown
Member

Hi - please let me know when updates have been made and I can review. No rush! Thank you.

@ldaws003
ldaws003 self-requested a review October 3, 2026 07:37
@ldaws003

ldaws003 commented Oct 3, 2026

Copy link
Copy Markdown
Member

Review ETA: 10/3/2026 EOD
Availability: Saturday Morning

ldaws003
ldaws003 previously approved these changes Oct 3, 2026

@ldaws003 ldaws003 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hi @KyleA99 , great job on the issue. Thanks for explaining the discrepancies from the mockup and the changes you made. That may require another issue to be made, but I'm not sure. Someone higher up would need to clarify. I'll approve.

@computarisis

Copy link
Copy Markdown
Member

Review ETA: Sat, Oct 3rd

Availability:
Everyday after 8PM ET

@computarisis computarisis left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hi, @KyleA99! Great job getting this issue going. I was able to validate your branch locally. I did notice the styling discrepancies that have been observed, and I'll wait until it is confirmed whether or not you can add your own css classes (I'd assume so). One little thing, I noticed the sidebar nav menu is no longer sticky for me... Could you double check that?

@castillios

This comment was marked as resolved.

@KyleA99

KyleA99 commented Oct 3, 2026

Copy link
Copy Markdown
Member Author

Hi @KyleA99 and reviewers. Regarding the discrepencies and the ability to create custom CSS classes, I'll bring this up to a lead for further review.

@castillios On the Tuesday Dev meeting last week @daras-cu mentioned I could add custom css classes to the "about" related scss files so my feature branch aligns with the mock up more. I was going to add that based off our Figma style guide. Should I hold off on that until I hear from you?

@castillios

Copy link
Copy Markdown
Member

@castillios On the Tuesday Dev meeting last week @daras-cu mentioned I could add custom css classes to the "about" related scss files so my feature branch aligns with the mock up more. I was going to add that based off our Figma style guide. Should I hold off on that until I hear from you?

Hi @KyleA99, I apologize for the confusion, please disregard my previous comment. I was unable to attend the previous dev meetings, so I wasn't sure if this was addressed yet. Glad to hear it was worked out and thanks for your continued work on this issue!

@KyleA99

KyleA99 commented Oct 3, 2026

Copy link
Copy Markdown
Member Author

@castillios On the Tuesday Dev meeting last week @daras-cu mentioned I could add custom css classes to the "about" related scss files so my feature branch aligns with the mock up more. I was going to add that based off our Figma style guide. Should I hold off on that until I hear from you?

Hi @KyleA99, I apologize for the confusion, please disregard my previous comment. I was unable to attend the previous dev meetings, so I wasn't sure if this was addressed yet. Glad to hear it was worked out and thanks for your continued work on this issue!

No problem! Appreciate the help. I will be working on that this weekend, and appreciate the reviews and patience from everyone :D

@KyleA99
KyleA99 dismissed stale reviews from ldaws003 and egcuriel via b7f40bb October 4, 2026 07:14
@KyleA99

KyleA99 commented Oct 4, 2026

Copy link
Copy Markdown
Member Author

@nathanjkim-codes @egcuriel @anthonylo87 @ldaws003 @computarisis I pushed changes (and re-requested reviews) if you all are able to review again.

  • The subheaders and faint "dividing" line are now styled better (I tried to utilize hackforla custom css properties and our Figma style guide).
  • I still don't see a SVG for the locations card in /assets/images/about/section-header-elements/
  • The image’s corners are still defaulting to black. The 3 images (Westside, Downtown L.A., and South L.A.) appear to inherently have the “black corners”. The border-radius which should be applied in css, is actually inherent to the images themselves. As a result, I can’t remove these black corners. I recommend that we either apply a border-radius in css and have raw “square corner” images, or to modify these images to be rounded.
  • Images are now in one row and appear to be flexing/responsive in a more ideal way.

@computarisis I saw your comment above regarding the sidebar/nav, but this does appear to be stick for me. Are you talking about this component?

Screenshot 2026-10-04 at 2 20 36 AM

@egcuriel

egcuriel commented Oct 6, 2026

Copy link
Copy Markdown
Member

Hi @KyleA99, thank you for the update

Review ETA: 10/07 EOD
Availability: W-Sun (8 pm - 11 pm)

@nathanjkim-codes

Copy link
Copy Markdown
Member

Review ETA: 10/07 EOD
Availability: Tuesday and Wednesday, 9:00 PM – 11:00 PM PT.

@anthonylo87 anthonylo87 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Hi @KyleA99,

Going to approve this now - but I think a lead needs to look into this and provide a bit more context/open a subsequent issue, as there are still outstanding items to address as you've captured (SVG is missing, location images already have rounded corners/black background.

@computarisis

Copy link
Copy Markdown
Member

Review ETA: Thu, Oct 8th

Availability:
Everyday after 8PM ET

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Complexity: Medium HLC: M Homepage Launch Countdown Must Have P-Feature: About Us https://www.hackforla.org/about/ P-Feature: Events https://www.hackforla.org/events/ role: back end/devOps Tasks for back-end developers role: front end Tasks for front end developers size: 1pt Can be done in 4-6 hours

Projects

Status: PRs ✅ waiting for merge team

Development

Successfully merging this pull request may close these issues.

Dev: Add the Our Locations Information to the About Page

7 participants