Skip to content

Manchester | 26-ITP-May | Monsur Abdulrahman | Sprint 2 | Wireframe - #1241

Closed
Monsur0001 wants to merge 8 commits into
CodeYourFuture:mainfrom
Monsur0001:feature/wireframe
Closed

Manchester | 26-ITP-May | Monsur Abdulrahman | Sprint 2 | Wireframe#1241
Monsur0001 wants to merge 8 commits into
CodeYourFuture:mainfrom
Monsur0001:feature/wireframe

Conversation

@Monsur0001

Copy link
Copy Markdown

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

Wireframe tasks completed.

@netlify

netlifyBot commented May 9, 2026

Copy link
Copy Markdown

Deploy Preview for cyf-onboarding-module ready!

NameLink
🔨 Latest commit44e6c0e
🔍 Latest deploy loghttps://app.netlify.com/projects/cyf-onboarding-module/deploys/6a1af8906eaf9f0008ade0ad
😎 Deploy Previewhttps://deploy-preview-1241--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: 96 (🔴 down 4 from production)
SEO: 91 (🟢 up 5 from production)
PWA: -
View the detailed breakdown and full score reports
🤖 Make changesRun an agent on this branch

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

@github-actions

This comment has been minimized.

@Monsur0001Monsur0001 added 📅 Sprint 1 Assigned during Sprint 1 of this module Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. Module-Onboarding The name of the module. labels May 9, 2026
@github-actions

This comment has been minimized.

@github-actionsgithub-actionsBot removed the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label May 9, 2026
@github-actions

This comment has been minimized.

1 similar comment
@github-actions

This comment has been minimized.

@Monsur0001Monsur0001 added Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. NotCoursework labels May 9, 2026
@illicitonionillicitonion changed the title Manchester | 26-ITP-May | Monsur Abdulrahman | Sprint 1 | WireframeManchester | 26-ITP-May | Monsur Abdulrahman | Sprint 2 | WireframeMay 9, 2026
@SambuxcSambuxc added the Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. label May 12, 2026

@SambuxcSambuxc left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

So far so good but a few things need addressing. Please take a look at my comments

Comment threadWireframe/index.html Outdated
<p class="center">Practicing commits in Git</p>
</header>
<main>
<div class="container">

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is there a semantic tag you could use instead here?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thank you so much for your feedback. I have switched back to the "main" semantic tag.

Comment threadWireframe/index.html Outdated
</main>
</div>

<div class="article-row">

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Is there a semantic tag you could use instead here?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thank you so much for your feedback. I have also switched this back to the "main" semantic tag.

Comment threadWireframe/index.html
<!DOCTYPE html>
<!doctype html>
<html lang="en">
<head>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Your page head should include a title and description

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thank you so much for your feedback. I have added the description as suggested.

Comment threadWireframe/style.css
margin: 0 auto calc(var(--space) * 4) auto;
}
footer {
position: fixed;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why did you decide to remove the fixed position?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thank you so much for your feedback. I noticed that when I leave the fixed position, I realized that my footer aligns back to the left instead of staying at the center. However, after deep research about the issue, I found that adding a width of 100% would easily solve the problem. I have made the correction immediately.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

It looks much better now, however the text becomes unreadable when scrolling at the background shows the page content through it.
Can you think of a way to make sure the text either; does not overlap the page content when scroll, or has an appropriate background color to make the footer text always readable?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I really do appreciate your feedback. I have fixed the readability issue by adding a white background to the footer. I also added a padding to it to solve the overlapping issue, so the users can actually see the last line of the content.. Thank you so much.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is looking really great now :)

Comment threadWireframe/index.html
<!DOCTYPE html>
<!doctype html>
<html lang="en">
<head>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

How have you linked your html to the style.css file?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thank you for your feedback. I have linked my css with this line of code ""

@Monsur0001

Copy link
Copy Markdown
Author

I really appreciate your feedback on my pull request. I have made all the suggested corrections and I am looking forward for your further feedback on it. Thank you so much.

@Monsur0001
Monsur0001 requested a review from SambuxcMay 13, 2026 14:34

@Monsur0001Monsur0001 left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I have done all the suggested corrections. Thank you so much as I anticipate your further feedback

@SambuxcSambuxc 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. Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels May 13, 2026
@Monsur0001Monsur0001 added 📅 Sprint 2 Assigned during Sprint 2 of this module and removed 📅 Sprint 1 Assigned during Sprint 1 of this module labels May 14, 2026
@Monsur0001Monsur0001 added the Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. label May 15, 2026
@JaypeeLan

Copy link
Copy Markdown

I can see this PR has both "need review" and "reviewed" labels. Do you still need it to be reviewed?

@Monsur0001

Copy link
Copy Markdown
Author

I can see this PR has both "need review" and "reviewed" labels. Do you still need it to be reviewed?

Yes please. I have made the corrections pointed out by the former reviewer. That’s the reason I added the needs review label. Thank you so much

@SambuxcSambuxc left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This is looking great now, well done :)

@SambuxcSambuxc added Complete Volunteer to add when work is complete and all review comments have been addressed. and removed Needs Review Trainee to add when requesting review. PRs without this label will not be reviewed. Reviewed Volunteer to add when completing a review with trainee action still to take. labels May 27, 2026
@illicitonion

Copy link
Copy Markdown
Member

Closing PR because the May ITP run has finished. Feel free to re-open if you're still working on it.

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

Labels

CompleteVolunteer to add when work is complete and all review comments have been addressed.Module-OnboardingThe name of the module.📅 Sprint 2Assigned during Sprint 2 of this module

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@Monsur0001@JaypeeLan@illicitonion@Sambuxc