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

Make AdminLessonNav more generic #2272

Merged
merged 5 commits into from
Sep 10, 2022
Merged

Make AdminLessonNav more generic #2272

merged 5 commits into from
Sep 10, 2022

Conversation

bryanjenningz
Copy link
Collaborator

@bryanjenningz bryanjenningz commented Sep 8, 2022

Closes #2268

This pull request changes:

  • AdminLessonNav is now more generic and called NavCard
  • NavCard takes in more simplified props
  • Makes it easier for the DOJO exercises page to use this NavCard component (Add DOJO exercises nav card #2265)

How to test:

  • Go to /admin/lessons/js1/modules
  • Verify that NavCard works and looks exactly how it looked and worked before

navcard

@vercel
Copy link

vercel bot commented Sep 8, 2022

@bryanjenningz is attempting to deploy a commit to the c0d3-prod Team on Vercel.

A member of the Team first needs to authorize it.

@vercel
Copy link

vercel bot commented Sep 8, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
c0d3-app ✅ Ready (Inspect) Visit Preview Sep 10, 2022 at 1:41AM (UTC)

@codecov
Copy link

codecov bot commented Sep 8, 2022

Codecov Report

Merging #2272 (c708bfd) into master (1784a86) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master     #2272   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          177       177           
  Lines         3010      3010           
  Branches       801       801           
=========================================
  Hits          3010      3010           
Impacted Files Coverage Δ
components/NavCard.tsx 100.00% <100.00%> (ø)
...es/admin/lessons/[lessonSlug]/[pageName]/index.tsx 100.00% <100.00%> (ø)

@@ -12203,6 +12163,44 @@ exports[`Storyshots Components/ModalCard Small 1`] = `
</button>
`;

exports[`Storyshots Components/NavCard Basic 1`] = `
Copy link
Collaborator Author

@bryanjenningz bryanjenningz Sep 8, 2022

Choose a reason for hiding this comment

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

This snapshot is identical to the snapshot that got removed above with the only difference being the name is now "Storyshots Components/NavCard Basic 1" instead of "Storyshots Components/AdminLessonNav Basic 1" and the classNames now use the more generic "navCard__" prefix instead of "lesson__".

I figured the name NavCard made more sense because it's a Card and it also contains Nav items and when I searched NavCard (https://github.com/garageScript/c0d3-app/search?q=NavCard), this name wasn't taken so it seemed like a nice short name that describes what it is pretty well.

background-color: color.change(variables.$primary, $lightness: 95%);
color: $dark_primary;
transition: background-color 0.3s ease-out;
}

.lessons_tabs {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This .lessons_tabs CSS class wasn't being used (https://github.com/garageScript/c0d3-app/search?q=lessons_tabs) so I deleted it.

@bryanjenningz bryanjenningz merged commit 5a78298 into garageScript:master Sep 10, 2022
@bryanjenningz bryanjenningz deleted the make-admin-lesson-nav-generic branch September 10, 2022 03:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make AdminLessonNav generic so it can be reused
3 participants