Show cards on curriculum catalog page - #51249
Conversation
|
Some follow-ups:
|
yeah, we'll need to decide how exactly to compute this because we'll be using the supported_locales field on the Unit model. this is easy when there is one unit in the course offering. when there are multiple units, I would imagine we want to mark it as supported only if all units are supported in that language. I think that would be a product question in the end though. |
I think the component I had used for it is not as responsive I guess, I'll look into what Kelby was using and switch over to that so it'll have better responsiveness! Thank you for catching this! |
Got it, thanks! Ticket for this work is here https://codedotorg.atlassian.net/browse/ACQ-514 |
Italian and German are my go-to's for testing long text :-) |
| display: -webkit-box; | ||
| overflow: hidden; | ||
| -webkit-line-clamp: 2; | ||
| -webkit-box-orient: vertical; |
There was a problem hiding this comment.
all supported browsers now recognizing a -webkit style! what a time to be alive 😁
There was a problem hiding this comment.
Ha! Amen. I had to read that documentation over and over to make sure I was reading it right ...
| oldestGrade={gradeLevelArray[gradeLevelArray.length - 1]} | ||
| subjects={school_subject?.split(',')} | ||
| topics={cs_topic?.split(',')} | ||
| isTranslated={!!Math.round(Math.random())} // TODO [MEG]: actually pass in this data |
There was a problem hiding this comment.
is this here so you can test both ways? this could be a little confusing to someone trying to repro a bug, so I'd prob recommend hard-coding it, and just using the react browser tool to toggle the prop value if you want to be able to see it change in the browser. no need to go back through drone just for this, though.
There was a problem hiding this comment.
That's a good point –– I'll update this in the next PR
There was a problem hiding this comment.
It was more for "easy" screenshots that would include the various options. But yes, I'll remove this in the next PR.
| .map(curriculum => curriculum.display_name) | ||
| .forEach(courseName => screen.getByRole('heading', {name: courseName})); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
these look like a good start and a good stopping point 👍
|
@megcrenshaw Following up on the grade question. I think we need to support cards with both 1 grade and 2 or more grades as CSF Course F has just one grade right now. |
Got it, will update in the next PR! |
4 participants
Puts the curriculum cards onto the Curriculum Catalog page. See it at http://localhost-studio.code.org:3000/catalog
Responsive (minus the banner –– see comment):
https://user-images.githubusercontent.com/9142121/231229618-53de77e5-2b7a-4a71-8e15-b7a0cb3265c6.mov
There are several todo's added as comments in the code –– syncing up with Turner and Dani to figure out what's already tracked and what needs a ticket.
A couple of design improvements were part of this work:
rtl. Some styling adjustments were made to ensure the RTL looks normal.Links
The creator of the React Testing Library suggests testing the parent component only (so I'd remove the tests from Curriculum Catalog Card and put them into the Curriculum Catalog). However, by the time we test all the logic for the filtering and the assign and quick view logic, they'd become super large files. I'm keeping it as-is for now (keeping them separate), but open to more conversation about this.
Testing story
For testing, used React Testing Library. Added Redux Toolkit to our repo because
createStoreis deprecated:Deployment strategy
Follow-up work
Privacy
Security
Caching
PR Checklist: