Skip to content

NW | ITP_MAY_25 | Nor_ZEHHAF | Wireframe_to_Webcode #408

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.

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

nor-cyf
Copy link

@nor-cyf nor-cyf commented May 10, 2025

Learners, PR Template

Self checklist

  • I have committed my files one by one, on purpose, and for a reason
  • I have titled my PR with REGION | COHORT_NAME | FIRST_NAME LAST_NAME | PROJ_NAME
  • I have tested my changes
  • My changes follow the style guide
  • My changes meet the requirements of this task

Changelist

Briefly explain your PR.

Questions

Ask any questions you have for your reviewer.

Copy link

netlify bot commented May 10, 2025

Deploy Preview for cyf-onboarding-module ready!

Name Link
🔨 Latest commit a5d6cd8
🔍 Latest deploy log https://app.netlify.com/sites/cyf-onboarding-module/deploys/681f680b6b798100080c356a
😎 Deploy Preview https://deploy-preview-408--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: 88 (🔴 down 12 from production)
Best Practices: 100 (no change from production)
SEO: 82 (🔴 down 4 from production)
PWA: -
View the detailed breakdown and full score reports

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

@nor-cyf nor-cyf added the Needs Review Participant to add when requesting review label May 10, 2025
@illicitonion illicitonion added Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. and removed Needs Review Participant to add when requesting review labels May 10, 2025
Copy link
Member

@illicitonion illicitonion left a comment

Choose a reason for hiding this comment

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

In terms of layout, this page doesn't quite match the wireframe. A few things:

  • The title and short description are meant to be centred, and the short description should probably be shorter.
  • I expect some empty space around the three articles, right now they take up the whole width of the page.
  • The git image is very large, and makes the page hard to interact with.
  • I ran lighthouse on this page and it didn't score 100.

<h3>Branches in Git</h3>
<address>
Written by a shared effort,
<time datetime="2025-05-09">on 2025/05/09</time>
Copy link
Member

Choose a reason for hiding this comment

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

A time of writing doesn't feel like it belongs in an <address> tag - can you think of a more suitable tag to use for this information?

(This applies to all three articles)

--font: 100%/1.5 system-ui;
--space: clamp(6px, 6px + 2vw, 15px);
--line: 1px solid;
--container: 1280px;
--container: 1250;
Copy link
Member

Choose a reason for hiding this comment

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

What are the units of this value?

body {
background: var(--paper);
color: var(--ink);
color: var(--ink);
Copy link
Member

Choose a reason for hiding this comment

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

Try to avoid adding trailing space where it's not needed.

</footer>
Branching in Git is a powerful feature that allows developers to diverge from the main codebase and work on new features, bug fixes, or experiments in isolation. By creating a separate branch, you can make changes without affecting the stable version of your project, enabling parallel development and smoother collaboration among team members. This article explores how branching works in Git, why it's essential in modern version control workflows, and how to effectively use branches to manage your code with confidence and flexibility.</p> <a href="https://git-scm.com/book/en/v2/Git-Branching-Branches-in-a-Nutshell">Read more</a>
</article>
<article id="second_article_wrap">
Copy link
Member

Choose a reason for hiding this comment

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

Why don't the second and third articles have images?

@illicitonion illicitonion added Reviewed Volunteer to add when completing a review and removed Review in progress This review is currently being reviewed. This label will be replaced by "Reviewed" soon. labels May 10, 2025
Copy link
Member

@illicitonion illicitonion left a comment

Choose a reason for hiding this comment

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

Also, your PR description template doesn't appear to have been filled in. Please edit the PR description to fill in all of the requested information.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Reviewed Volunteer to add when completing a review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants