-
Notifications
You must be signed in to change notification settings - Fork 40
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
[APT-1585] Courses page #99
Conversation
✔️ Deploy Preview for pensive-meitner-faaeee ready! 🔨 Explore the source changes: e8212b4 🔍 Inspect the deploy log: https://app.netlify.com/sites/pensive-meitner-faaeee/deploys/61ba26ac87e74e00083f4863 😎 Browse the preview: https://deploy-preview-99--pensive-meitner-faaeee.netlify.app |
This is now ready for review. Here's the direct link to the page. Note that there are some minor discrepancies between this and the mockup in Figma. Most notably, the page width and margins aren't the same. That's an artifact of utilizing the built-in Docusaurus layout, which we want to do since it allows us to utilize markdown (including the admonition) for the rest of the page. I'm okay with that tradeoff. I briefly explored adjusting the style conditionally for pages without sidebars, but I wasn't able to account for presence or absence of the TOC in the selector, so it proved near-impossible to achieve. |
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.
It looks pretty darn good - but I wonder if we can get the content centered?
I see that you're hiding the the TOC - but I'm wondering if we need more of a centered layout for this page.
Edit: I see that you're explicitly NOT hiding the TOC but making use of the fact that we've modified the component. I wonder if in this case - we want something else. Perhaps this is where we use our own Layout?
Custom layouts don't appear to be available to us just yet. I would love to center it, but found no way to do so that wouldn't interfere with other pages, or result in an extra wide layout (when hiding the TOC). |
We found a solution for centering by adding an MDX wrapper to the content. |
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.
LGTM!
This is a solid start towards the dedicated courses page. I've chosen to include the page underneath
/docs/guides
as a.mdx
file, which enable us to utilize built-in layouts and markdown on the page in conjunction with the Courses component.