-
Notifications
You must be signed in to change notification settings - Fork 10
add tutorials to language page and create SplitCard component #485
Conversation
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/ocaml/v3-ocaml-org/FoUSkjjqGA6vEUhysChzQ1s3qayP |
You can see these changes, plus the changes to the books component at the same time using https://v3-ocaml-org-git-fork-patricoferris-fix-books-ocaml.vercel.app |
<Route _to={#resourcesTutorial(tutorial.slug)} lang> | ||
<a className="text-orangedark"> {s("Read more...")} </a> | ||
</Route> | ||
</div> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Feel free to generalize this card as an abstract card in the component library, if you see potential for reuse.
I'm not sure if you wanted a review or if you wanted me to look into the html or css in depth. I proactively added some comments. I didn't look at the html and css in depth, but the overall structure of the code looks good. |
@@ -10,7 +10,7 @@ let make = (~cardData, ~renderCard, ~title=?, ()) => <> | |||
</h2> | |||
| None => <> </> | |||
}} | |||
<div className="grid grid-cols-1 lg:grid-cols-2 gap-x-40 gap-y-4 px-8"> | |||
<div className="grid grid-cols-1 lg:grid-cols-2 gap-x-28 gap-y-4 px-8"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider introducing a variant with values "Wide | Normal" or similar, to allow the user to select this value.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your reviews are always more than welcome (and very useful), I've created a couple of follow-up issues. Merging now! :) |
Changes
SplitCard
component that is used on the language page and the events page (at the moment)Discussion
This PR adds access to the tutorials we now have in ood. It partially completes #186 but the design is not identical. I was in favour of trying to converge on a common set of components by implementing a
SplitCard
component which I have then reused over on the events page too (which hopefully proves its usefulness).The tutorial index page is incredibly simple, and just reuses the
CardGrid
, we'll probably want to redesign this page (I don't believe we have a design just for it) to make use of the proficiency levels and tags for better organisation. But for now I'm just in favour of getting a "clickable" way to get to any of the tutorials :))Screenshots
The call to action section of the language page, it just displays the first few tutorials and then links to the index.
The index page for the tutorials, nothing fancy 😅