Skip to content
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

fix(client): map progress ui #54576

Merged
merged 7 commits into from
May 1, 2024
Merged

Conversation

ahmaxed
Copy link
Member

@ahmaxed ahmaxed commented Apr 29, 2024

Checklist:

  • I have read freeCodeCamp's contribution guidelines.
  • My pull request has a descriptive title (not a vague title like Update index.md)
  • My pull request targets the main branch of freeCodeCamp.
  • I have tested these changes either locally on my machine, or GitPod.
Screenshot 2024-04-29 at 11 29 01 PM

@ahmaxed ahmaxed requested a review from a team as a code owner April 29, 2024 20:27
@github-actions github-actions bot added the platform: learn UI side of the client application that needs familiarity with React, Gatsby etc. label Apr 29, 2024
Copy link
Member

@moT01 moT01 left a comment

Choose a reason for hiding this comment

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

Thanks for jumping on this @ahmaxed. I gave it a quick look - here's what I see:

Firefox:
Screenshot 2024-04-29 at 9 45 31 PM

Safari:
Screenshot 2024-04-29 at 9 45 12 PM

Looks good in Chrome. I played with it a bit - couldn't quite get it right, but have some ideas. Let me know if you want to pair up on it for a bit. Since deployments are sort of blocked until we get this looking a little better, maybe it's okay to just have it look good in Chrome temporarily if we need to.

I also kind of feel like only the core curriculum needs the numbers. Not sure if we should just omit the icons for the rest - I think I would be fine with that - or we could maybe use some neutral icons, like just a circle without the number, and the ribbons if isCert

@ahmaxed
Copy link
Member Author

ahmaxed commented Apr 30, 2024

We can definitely pair on it. I agree, the items outside of the curriculum set should not have numbers or arrows.

@ahmaxed
Copy link
Member Author

ahmaxed commented Apr 30, 2024

Screenshot 2024-04-30 at 1 49 35 PM

@Sembauke
Copy link
Member

The arrows seem no longer centred.

@moT01
Copy link
Member

moT01 commented Apr 30, 2024

I added some logic to see if the user has completed all the challenges in a superblock - if so, the icon with the double circle shows up:

Screenshot 2024-04-30 at 11 51 46 AM

^
1: none
2: isCert
3: isCert & allChallengesCompleted
4: same as 2
5: allChallengesCompleted

@moT01
Copy link
Member

moT01 commented Apr 30, 2024

Not sure if we want this, but I made the arrows a little thicker too:

Screenshot 2024-04-30 at 12 08 49 PM

Let me know if I should revert

@ahmaxed
Copy link
Member Author

ahmaxed commented May 1, 2024

Tom's changes are LGMT.

Copy link
Member

@ShaunSHamilton ShaunSHamilton left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Though, the arrows do not look like arrows:
image

@ShaunSHamilton ShaunSHamilton changed the title fix: map progress ui fix(client): map progress ui May 1, 2024
@moT01 moT01 merged commit c075247 into freeCodeCamp:main May 1, 2024
22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
platform: learn UI side of the client application that needs familiarity with React, Gatsby etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants