Skip to content

West-Midlands | 26-ITP-Jan | Adedolapo Bamiduro | Sprint 1 | Wireframe #1129

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.

Sign up for GitHub

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

Jump to bottom
Open
codebydolz wants to merge 5 commits into
base: main
Choose a base branch
from
Open

West-Midlands | 26-ITP-Jan | Adedolapo Bamiduro | Sprint 1 | Wireframe #1129

codebydolz wants to merge 5 commits into from
+69 −26

Conversation

@codebydolz
Copy link

@codebydolz codebydolz commented Feb 4, 2026

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 have made the added changes as per instructions

@netlify
Copy link

netlify bot commented Feb 4, 2026 •
edited
Loading

✅ Deploy Preview for cyf-onboarding-module ready!

Name Link
🔨 Latest commit 121df04
🔍 Latest deploy log https://app.netlify.com/projects/cyf-onboarding-module/deploys/6989c11065dcbf00089f3c45
😎 Deploy Preview https://deploy-preview-1129--cyf-onboarding-module.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse		 2 paths audited
Performance: 100 (no change from production)
Accessibility: 100 (no change from production)
Best Practices: 100 (no change from production)
SEO: 91 (🟢 up 5 from production)
PWA: -
View the detailed breakdown and full score reports

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

@codebydolz codebydolz added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label Feb 4, 2026
cjyuan
cjyuan reviewed Feb 6, 2026
View reviewed changes
Copy link
Contributor

@cjyuan cjyuan left a comment

Choose a reason for hiding this comment

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

Can you take a look at this General Feedback to see if there
is anything you can do to make your PR more robust and ready?
Doing so can help speed up the review process.

@cjyuan cjyuan added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Feb 6, 2026
@codebydolz codebydolz added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Feb 7, 2026
cjyuan
cjyuan reviewed Feb 7, 2026
View reviewed changes
Copy link
Contributor

@cjyuan cjyuan left a comment

Choose a reason for hiding this comment

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

  • According to https://validator.w3.org/, there are errors in your code. Can you fix them?

  • One of the acceptance criteria, "The page footer is fixed to the bottom of the viewport", has not yet been satisfied. Can you make the necessary change? (Suggestion: Ask ChatGPT what that requirement means).

  • When a wireframe is provided, our implementation should closely reflect its appearance and layout to ensure consistency with design expectations. You're off to a solid start. To better align with the wireframe, can you

    • Left justify the article content and the "Read more" links (instead of center them)
    • Make the "Read more" link in article 1 on separate row.
      Right now it appears on the same row as the article summary.
Image

Wireframe/index.html Outdated
</article>

<article>
<img src="placeholder.svg" alt="an image depicting wireframe " />
Copy link
Contributor

@cjyuan cjyuan Feb 7, 2026

Choose a reason for hiding this comment

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

Why leave a trailing space in the alt value?

Copy link
Author

@codebydolz codebydolz Feb 8, 2026

Choose a reason for hiding this comment

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

i think i have fixed it @cjyuan

Copy link
Contributor

@cjyuan cjyuan Feb 9, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

There is still a trailing space in he alt value.

@cjyuan cjyuan added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Feb 7, 2026
This was referenced Feb 8, 2026
Closed
Closed
@codebydolz codebydolz added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Feb 8, 2026
cjyuan
cjyuan reviewed Feb 9, 2026
View reviewed changes
Copy link
Contributor

@cjyuan cjyuan left a comment •
edited
Loading

Choose a reason for hiding this comment

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

  • When a wireframe is provided, our implementation should closely reflect its appearance and layout to ensure consistency with design expectations. To better align with the wireframe, can you

    • Center the SHORT DESCRIPTION beneath the title
    • Center the content in the footer
    • Leave more space around the "Read more" link in article 1 (ideally the same amount of space you used in articles 2 and 3)

  • There are some typos in index.html.

    • Suggestion: Share your code to an AI tool and ask it to review your code in terms of semantic, accessibility, typo, and consistency.
      It can often help us catch errors, improve consistency, and reinforce best practices.

  • Currently, the content in the footer and the content in the articles can overlap. Can you find a way to prevent it from happening? (note: The footer must stay fixed at the bottom of the viewport)

image

Wireframe/index.html Outdated
</article>

<article>
<img src="placeholder.svg" alt="an image depicting wireframe " />
Copy link
Contributor

@cjyuan cjyuan Feb 9, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

There is still a trailing space in he alt value.

@cjyuan cjyuan added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Feb 9, 2026
@codebydolz
Copy link
Author

codebydolz commented Feb 9, 2026

thank you for the corrections. i have just created a new commit

@cjyuan
Copy link
Contributor

cjyuan commented Feb 9, 2026 •
edited
Loading

The content in the footer and the content in the articles can still overlap. Can you find a way to prevent it from happening? (note: The footer must stay fixed at the bottom of the viewport)

image

And don't forget to add a "Needs Review" label whenever your PR is ready to be re-reviewed.

@codebydolz codebydolz added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. and removed Reviewed Volunteer to add when completing a review with trainee action still to take. labels Feb 10, 2026
@cjyuan
Copy link
Contributor

cjyuan commented Feb 10, 2026

I don't see any new commit on this branch. If you have made commits on your computer, you need to push them to GitHub for them to appear on this PR.

@cjyuan cjyuan added Reviewed Volunteer to add when completing a review with trainee action still to take. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. labels Feb 10, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cjyuan cjyuan cjyuan left review comments

At least 2 approving reviews are required to merge this pull request.

Assignees

No one assigned

Labels

Reviewed Volunteer to add when completing a review with trainee action still to take.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@codebydolz @cjyuan