Apply Title heading class to Events page #2081

[lindseyindev]

Fixes #2081

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

-Added appropriate Title classes to events page headings per figma design file

-Updated heading elements to be more semantically correct for screen readers because the styling is now through the Title class.

• Added color property to title-meetup class in /_events.scss set to #333333 per figma design file because there was a universal h4 color set to pink that was being applied to the title5 elements on the meetup events column

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

Visuals before changes are applied

Visuals after changes are applied

[github-actions]

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 lindseyindev-apply-heading-classes-2081 gh-pages
git pull https://github.com/lindseyindev/website.git apply-heading-classes-2081

[Georgema20]

Everything looks good, but when I pull it up, there is a visual change in the left column as a result of the class change put in onboarding and those titles. Please make sure to reflect this properly in the visuals after change section with screenshots. Also, please put in a before screenshot as well.

Everything else looks pretty good. Keep it up! Make the changes then I can approve PR @lindseyindev

[lindseyindev]

@Georgema20 thank you for letting me know, I updated the PR comment for your review!

[Georgema20]

Nice work!

[SAUMILDHANKAR]

Review ETA: EOD 1/5/22
Availability: 6 hrs

[SAUMILDHANKAR]

@lindseyindev Hi Lindsey, thanks for taking up this issue. I have been comparing your events page with that at Figma and I see that the titles onboarding, workshop, happy hour should be dark gray whereas in your branch they don't appear so. As per the design in Figma, they should be dark gray.

[lindseyindev]

I updated the scss for the title-meetup class which applies to just those titles that were showing as pink from styling on the h4 element, I updated the color to #33333 per the figma file and I also updated the after picture in my submission!

[SAUMILDHANKAR]

Revised review ETA: EOD 1/8
Availability: Saturday 2-5 PM
Couldn't finish the review as per this estimate

[SAUMILDHANKAR]

Revised review ETA: EOD 1/10
Availability: Monday 2-4 PM

[SAUMILDHANKAR]

title1, title2 etc in lower case.

[SAUMILDHANKAR]

I updated the scss for the title-meetup class which applies to just those titles that were showing as pink from styling on the h4 element, I updated the color to #33333 per the figma file and I also updated the after picture in my submission!

Great job in catching the color of h4. As per design system on Figma, text color should be black or white and red is reserved for links, so just wondering if we should remove red color from h4. This can be part of future issue, we can confirm with design in sunday meeting.

[lindseyindev]

Updated the title classes to be lower case & submitting for re-review!

Would definitely recommend a suggestion for design & for other new developers, like how we mentioned in the meeting differences between design and codebase, that the figma class should be text-cased to match what will be coded especially if that's the reference for devs
or it's noted in the issue if for some reason figma classes can't be lowercased

[JessicaLucindaCheng]

@lindseyindev Thanks for working on this issue for so long! In addition to my inline comments, could you update your before and after images with a full screen shot of the page?

You can use this extension: https://chrome.google.com/webstore/detail/gofullpage-full-page-scre/fdpohaocaechififmbbbbbknoalclacl?hl=en

[JessicaLucindaCheng]

@lindseyindev Please provide your availability and ETA for working on the requested changes above.

[lindseyindev]

@lindseyindev Please provide your availability and ETA for working on the requested changes above.

I did not see this until now, I should have this updated within 24 hours!!!

[JessicaLucindaCheng]

@lindseyindev Thanks for making the requested changes! I think we are almost there with this pull request. Once you make changes based on my in-line request for a change, can you update your before and after images in your original post/comment? The before should look like the current webpage. Thank you for all your hard work!

[lindseyindev]

I didn't realize changing the h5 tag to h4 undid the previous fix I did to make the color adhere to the figma so I fixed it by targetting the day heading class.

Also somehow a pull request I reviewed is being added as a commit when I pushed my code even though that was on a separate branch and I never committed it

The code with the tags in the commit history.. #2718 not sure how to undo that on my end -- I dont see a way to remove that part of the commit

[JessicaLucindaCheng]

@lindseyindev

The code with the ** tags in the commit history.. #2718 not sure how to undo that on my end -- I dont see a way to remove that part of the commit**

I'm wondering if doing the following would fix the problem you are having:
i. Update your forked gh-pages so that it is in sync with the hackforla/website gh-pages. Here are instructions for how to update your forked gh-pages.
ii. Then merge your gh-pages into your topic branch locally.
iii. Then push the merged changes in your topic branch locally to your remote topic branch in your forked repo.

[lindseyindev]

I think it worked but I'm not entirely sure how.. like how pushing to my remote fork also pushes to this PR at the same time

[JessicaLucindaCheng]

• Availability: 6 hours
• Re-review ETA: Mon, Feb 7, 2022

[JessicaLucindaCheng]

@lindseyindev Thanks for updating your topic branch!

I think it worked but I'm not entirely sure how.. like how pushing to my remote fork also pushes to this PR at the same time

To answer your question, your pull request is connected to your topic branch:

Thus, any changes to your topic branch will show up in your pr.

Out of date