London | 26-ITP-Jan | Shuheda Begum | Sprint 1 | Wireframe

[codebyshay]

Learners, PR Template

Self checklist

• I have titled my PR with Region | Cohort | FirstName LastName | Sprint | Assignment Title
• My changes meet the requirements of the task
• I have tested my changes
• My changes follow the style guide

Changelist

I completed Wireframe with 100 accessibility score on Lighthouse
PR template completed and ready for review

[netlify]

✅ Deploy Preview for cyf-onboarding-module ready!

Name	Link
🔨 Latest commit	2b73779
🔍 Latest deploy log	https://app.netlify.com/projects/cyf-onboarding-module/deploys/6973ed0817dde00008272322
😎 Deploy Preview	https://deploy-preview-956--cyf-onboarding-module.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.
Lighthouse	2 paths audited Performance: 100 (no change from production) Accessibility: 100 (no change from production) Best Practices: 100 (no change from production) SEO: 86 (no change from production) PWA: - View the detailed breakdown and full score reports

---

To edit notification comments on pull requests, go to your Netlify project configuration.

[jenny-alexander]

Hi @codebyshay - your wireframe webpage looks very good! You have styled it nicely and made solid code changes within the files.

Here are a few adjustments that should be made (in addition to the comments I added in the code).

• The 'Read more' buttons should have an outline. You can refer to the exercise wireframe image for reference: https://github.com/CodeYourFuture/Module-Onboarding/blob/main/Wireframe/wireframe.png
• The footer should be fixed to the bottom of the page. This means that the footer should always be visible on the page. Here is a resource to review: https://www.w3schools.com/howto/howto_css_fixed_footer.asp
• Formatting you code within the IDE (VSCode, for example) can make your code easier to read for yourself and others. You can do this by right-clicking within a code file and choosing 'Format Document'.

[jenny-alexander]

The <img> src is very long and it makes your HTML hard to scan. Would it be possible for you to download the image locally and then save it within the project (you could create an /img folder within the Wireframe directory). This will make the code easier to read and maintain!

The <img> alt text is not descriptive enough. Here is a good resource to review: https://accessibility.huit.harvard.edu/describe-content-images

[jenny-alexander]

The <img> src is very long and it makes your HTML hard to scan. Would it be possible for you to download the image locally and then save it within the project (you could create an /img folder within the Wireframe directory). This will make the code easier to read and maintain!

The <img> alt text is not descriptive enough. Here is a good resource to review: https://accessibility.huit.harvard.edu/describe-content-images

[jenny-alexander]

The <img> alt text is not descriptive enough. Here is a good resource to review: https://accessibility.huit.harvard.edu/describe-content-images

[codebyshay]

Hi @jenny-alexander
Thank you for your feedback. I have gone through each point and amended as required. Please have a look,

Shuheda

[jenny-alexander]

Some good adjustments were made.

1. Read More links:
   I see you added a border around the Read more link so it looks more like a button. However, now there isn't enough padding around the words and it looks very squishy.
   The original styling of the button seems like it would be good to use.
   🤔 Perhaps you can revert to this?

a {
  padding: var(--space);
  border: var(--line);
  max-width: fit-content;
}

2. The footer is now fixed in place👍

There is a slight issue with being able to see the footer text when I scroll. If you add a background color to your footer, you will be able to see it better.
In this video, I show how if you make the background color white and add a border-top (in CSS), it's much clearer. Can you try something similar?

Screen.Recording.2026-01-22.at.11.18.01.PM.mov

[jenny-alexander]

The <img> filename is incorrect here. Can you review it? They are case-sensitive.

[jenny-alexander]

Can you review the border-top? I am not sure the value '000' is.

[jenny-alexander]

There are two footer classes defined. It's easier to maintain and avoid conflicts if only one footer class is defined.

[jenny-alexander]

@codebyshay Don't forget to change the label to 'Needs Review' once you make your changes and want it reviewed again!

[codebyshay]

@jenny-alexander yes! Thank you for the reminder :)

[JaypeeLan]

The link is not working.

[jenny-alexander]

Hi @JaypeeLan - I already started to review this PR yesterday. Do you have a question about my review so far? If so, can you reach out on Slack so we don't add unnecessary comments to the PR? 🙂

[codebyshay]

@jenny-alexander Not sure what's going on, but this has been like this for over 30mins.

[jenny-alexander]

The footer is much more visible now. 👍

There are still some adjustments that need to be made within the style.css file. Please see the comments I made within there.

Can you please style the 'Read more' buttons so they line up horizontally?

[jenny-alexander]

Is z-index required here? If yes, what purpose does it serve?

[codebyshay]

I've spoken to a few others that have completed this task, an none of them have been asked to line up the 'Read more' horizontally. I will run the comments you've made by Ebrahim tomorrow morning, because I've done the task, its complete, but you keep coming back with more things to do. I've got many other mandatory task to complete, and at this rate, all my free time is spent correcting things, that the others I'm working alongside, have not even have to address and their work has been marked complete.

[jenny-alexander]

@codebyshay - I appreciate you raising this concern.
However, the requirement isn't something I've added—it's in the wireframe/specification that was provided at the start. The task is to build a page that matches the design, and the 'Read more' alignment is part of that design.

I have asked other ITP trainees to fix alignment issues with their wireframe webpage.

I completely understand you have other tasks and time constraints! If you feel the requirements are unreasonable or unclear, that's absolutely worth discussing with Ebrahim. But from a code review perspective, my job is to ensure the implementation matches the specification—which includes layout details like alignment.

[jenny-alexander]

What is 000 used for?

[codebyshay]

Possibly a mistake. I'll amend it.

[jenny-alexander]

We should use !important very sparingly when styling. It can make CSS difficult to maintain over time.
Here is some good documtation:
https://www.w3schools.com/css/css_important.asp
(Check out the 'Use !important Sparingly' section) 🙂

[codebyshay]

I appreciate the insight, but I can still use it right?

[jenny-alexander]

The '!important is used to override other CSS rules when there are competing styles. It makes no sense to use !important here since there are no other rules in your css files that target <a>.

[jenny-alexander]

Is !important really needed here? If yes, why?

[codebyshay]

I was getting desperate and impatient. So I added important to ensure the action happened.

[jenny-alexander]

I totally understand getting frustrated when CSS styling isn't working as expected!
However, reaching for !important when things aren't working is often a sign that we need to pause and investigate why the styles aren't applying, rather than forcing them through.

Using !important in this case is trying to mask the problem rather than fix it. In your case, there is no need to use !important

[jenny-alexander]

@jenny-alexander Not sure what's going on, but this has been like this for over 30mins.

I don't see this currently happening anymore. Let me know if it comes back!

[codebyshay]

@jenny-alexander oh ok, still loading on my end lol! I managed to go to the gym and back and its still loading for me! Are you able to confirm if my PR has successfully been sent?

[jenny-alexander]

@codebyshay At least you were productive while waiting for these jobs to do something! 💪

If I click on the ... next to one of those long-running jobs, select View Details. Then on that page, click on the link to view more details.

You can see it says "Deploy Successful". It says this for all 3 of those 'in progress' checks.

It's possible that GitHub isn't receiving the latest status of these checks. Maybe when you push your new set of commits, it will refresh the state.

It won't matter for your PR though! 😄

[jenny-alexander]

@codebyshay If you prefer to come back at some point and align those 'read more' buttons (as an exercise to further refine your CSS skills), that's okay with me.
I think removing the usage of !important will be good enough to mark this as complete.

If you still need some help regarding/understanding why we should preferably not use !important, you can ask your in-person mentor tomorrow - or ask AI?