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

Add templates to doc pages and give docs home one #1268

Merged
merged 18 commits into from
May 22, 2020
Merged

Conversation

rogermparent
Copy link
Contributor

This branch is now at the point where docs have configurable templates and the docs home has a unique template at src/templates/doc-home.tsx as opposed to regular docs which use src/templates/doc.tsx (before, both used doc-home).

Now that the scaffolding is there, we just have to actually make the page.

  • Default docs page template moved from src/templates/doc-home.tsx to
    src/templates/doc.tsx. A new homepage-specific (but currently identical)
    template has been added in its place.

  • Added frontmatter to docs home source to define the template

  • Docs now have a "template" field, which when defined changes the component
    used to make its page. This is a file relative to src/templates with the
    extension left off. This defaults to doc, or the file
    src/templates/doc.tsx. This is not currently on blog posts.

- Default docs page template moved from `src/templates/doc-home.tsx` to
  `src/templates/doc.tsx`. A new homepage-specific (but currently identical)
  template has been added in its place.

- Added frontmatter to docs home source to define the template

- Docs now have a "template" field, which when defined changes the component
  used to make its page. This is a file relative to `src/templates` with the
  extension left off. This defaults to `doc`, or the file
  `src/templates/doc.tsx`. This is not currently on blog posts.
@rogermparent rogermparent marked this pull request as draft May 8, 2020 18:20
@shcheklein shcheklein temporarily deployed to dvc-landing-new-docs-ho-9zukbm May 8, 2020 18:20 Inactive
src/templates/doc.tsx Outdated Show resolved Hide resolved
@shcheklein
Copy link
Member

@rogermparent what is the plan moving forward? I'm not sure how will we connect MD with this custom template. What kind of customization it will allow us?

@rogermparent
Copy link
Contributor Author

rogermparent commented May 9, 2020

The path forward from here is very open and subjective, as we have full freedom on how to connect the docs home's metadata with the new copied component.
There are a few implementations we could go with, depending on editors' preference.

Given a simple "blocks and some text content" layout, there's three broad directions we can take:

1. Define the homepage "blocks" in frontmatter, rendering them programmatically in the component.

Pros:

  • All content can be edited in the markdown page.
  • Once finished, the template rarely has to change.
  • Works best with Remark

Cons:

  • Frontmatter-heavy source file looks a little clunky.

2. Lean on the template component for everything that isn't the body text.

Pros:

  • Very simple implementation

Cons:

  • Page content is split between Markdown and Template
  • Worst for editors

3. Use custom rehype-react components for blocks
We already use rehype-react in docs so there's no worries about having to add it. Since this branch has a unique home template, we can choose to add custom components for either just this page or all pages.

Pros:

  • All content in one clean, understandable source file
  • Most flexible
  • Components are just React, no extra rehype/mark plugins required!
  • Useful for unique pages in the future

Cons:

  • Writing React in MD can feel a little weird.

I suppose it's a little obvious that I'm most for using the rehype-react approach to make the block menu out of custom components. Frontmatter-based is viable if editors prefer that way of making content. Leaning on the template and splitting the content defeats the purpose of using MD in the first place, so it's my least favorite option.

@shcheklein
Copy link
Member

@rogermparent if we go all components, will we need the md file at all? will it have empty body?

Also, if we go all components - will we still (on a component level) use Markdown component to render block of texts?

Probably, would be useful at this point to see an example?

@rogermparent
Copy link
Contributor Author

rogermparent commented May 10, 2020

@rogermparent if we go all components, will we need the md file at all? will it have empty body?
Also, if we go all components - will we still (on a component level) use Markdown component to render block of texts?

Using custom components is one of the only options that keeps content in the body. The text content with this method is written in the markdown file's body as mostly markdown while components are used to wrap special constructs like the home menu blocks.

Probably, would be useful at this point to see an example?

Absolutely! Making the example is my next top priority right now, I've just been taking it light on the weekend. I'll do one with the component syntax first.

This is not intended to be final, but show off the general premise of
how using custom components for this kind of layout would look like.

The card and cards components are, on top of being fully stylable with CSS
modules, capable of using arbitrary Markdown images or elements as the card
"logo" when the `logo` prop is set to `true` (or anything truthy, like the
string `"true"`). This works around the complexity of working in custom image
capability by leaning on the one we already have in Markdown.

To make a card with a logo, set the "logo" prop to true and make sure the first
element in the card contains the image you want- this element will be set aside
in its own div, with the rest of the contents displayed beside it using `flex`

There are some quirks with Remark custom components, though:

- custom components aren't indented

- custom components must be contained to one line, including props.

- custom components can include Markdown, but there must be an empty line
  between the transition.

- self-closing tags aren't supported

Should we use custom components more often, we can migrate to MDX (I actually
already have a fully working branch with it that I made on my own time) which
handles this syntax much more comfortably because it's designed around the
concept. All our custom remark-based logic will cleanly port because MDX
actually uses Remark in its pipeline.
@rogermparent rogermparent self-assigned this May 16, 2020
@rogermparent rogermparent marked this pull request as ready for review May 16, 2020 00:49
@rogermparent rogermparent marked this pull request as draft May 16, 2020 00:50
@rogermparent rogermparent temporarily deployed to dvc-landing-new-docs-ho-hfp42t May 16, 2020 00:53 Inactive
{Array.isArray(children)
? children.reduce(
(acc: Array<object>, child, i) =>
typeof child === 'object'
Copy link
Member

Choose a reason for hiding this comment

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

what else can it be ? do we ignore it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This filters out the "\n" strings that occur in remark custom component children, as well as anything else that isn't a card, as cards components should only contain card components.

Copy link
Member

Choose a reason for hiding this comment

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

may be it is safer to raise an exception?

Copy link
Member

Choose a reason for hiding this comment

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

at least if it something except \n

Copy link
Member

Choose a reason for hiding this comment

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

\s*

@@ -1,3 +1,7 @@
---
template: 'doc-home'
Copy link
Member

Choose a reason for hiding this comment

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

it looks like we made cards general enough, do we need a separate template now?

Copy link
Contributor Author

@rogermparent rogermparent May 16, 2020

Choose a reason for hiding this comment

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

You're right, I expected to need to do something in another component but everything went fine with the new custom components in the body.
I'll remove this template frontmatter and the existing home template. We may need a unique template in the future, but we'll add it when that time comes.

Copy link
Member

Choose a reason for hiding this comment

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

hey, any update to this? are we removing the template?

Copy link
Contributor Author

@rogermparent rogermparent May 19, 2020

Choose a reason for hiding this comment

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

I totally can, and quite quickly, but considering we don't know the exact direction this page is going I haven't committed a change to it yet.

Given my comment on the home issue, I think the best course of action is to temporarily go all-in on the home template and make it a standalone component. This allows us to not be limited in design and get the docs home we want up quickly.

My current plan is that once I get the design I'll make the template that's a React component implementing that design, and once that makes the Docs page match what we want, we merge to master- this first step of the plan lets us get a great docs home out before the important DVC 1.0 events around the corner because the implementation is already here and React components are the easiest to port to any other method down the line.

After we have a page we want made in a simple React component template, we can later quite easily port the page content to something more dev/editor friendly that lets editors change the page in MD.

Copy link
Member

Choose a reason for hiding this comment

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

@rogermparent I would be happy if we can get something simple and clean like this - https://cube.dev/docs . I don't think we need something too complicated/custom at this point. Does it help to make a decision?

Also, we can take design ideas there (except icons of course :) ) . I'll ask designer after that to make us icons.

}> = ({ children }) => {
return (
<div className={styles.cards}>
{Array.isArray(children)
Copy link
Member

Choose a reason for hiding this comment

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

I'm still not sure about this - if we have this check, we should have a different type above to my mind ... it is strange that ts-lint complains - any idea why is it happening?

In an effort to make the current solution good enough to fit release, I've
removed the alternate template since this branch currently favors custom
components.
@shcheklein shcheklein temporarily deployed to dvc-landing-new-docs-ho-jx8njb May 21, 2020 21:33 Inactive
@shcheklein
Copy link
Member

shcheklein commented May 21, 2020

Let's do the following as a next step (to merge it):

  • Remove icons for now
  • Make card clickable. It means one action (topic) per card cc @jorgeorpinel
  • Card should have a title

After that I'll be able to show this to a designer and get some actual design.

@rogermparent
Copy link
Contributor Author

rogermparent commented May 21, 2020

Let's do the following as a next step (to merge it):

I'm on it!

* [ ]  Remove icons for now

Easy enough

* [ ]  Make card clickable. It means one action (topic) per card cc @jorgeorpinel

Also easy, but we'll have to decide which one takes over for the cards that currently have two.

* [ ]  Card should have a title

Similar to the card links, the implementation is easy once we decide on what the titles are.

I'll get as far as I can with my own decisions as an example and adapt it when other feedback comes in.

Cards now accept a `href` prop, which turns the whole card into a link. This is
our Link wrapper, so we get the same automatic `rel` and `target` setting for
external links as well as Gatsby Link for internal ones.

This link spans the whole card, and has some styling to go along with it.
Currently the hover style is set to a slight grow animation, but it could be
anything.

Most of the `children`-modifying hackery has been removed in favor of wrapping
all Cards. This also brings a degree of consistency to Cards, and we can apply
indivual styles if the need arises. It's still needed for the Card Images,
however, since newlines that show up in `children` are mandatory for how
rehype's React and MD are separated.
@rogermparent rogermparent temporarily deployed to dvc-landing-new-docs-ho-jx8njb May 21, 2020 23:33 Inactive
@rogermparent
Copy link
Contributor Author

rogermparent commented May 21, 2020

Done with the design changes, and the Card components are now more sanely written.

@rogermparent rogermparent marked this pull request as ready for review May 22, 2020 00:23
@shcheklein shcheklein requested a review from jorgeorpinel May 22, 2020 02:31
@shcheklein
Copy link
Member

@jorgeorpinel let's keep Get started instead on Install and Python API reference instead of Use Cases (use cases are not ready to be honest to put them here)?

@shcheklein
Copy link
Member

@rogermparent : (sorry for pixel movements again) - let's for now do the same effect as we have for buttons to the right? It might be enough and will look consistent (I'll ask for icons after we decide on the content with @jorgeorpinel )

Now they match the right side buttons, aside from the animated transition.
@rogermparent rogermparent temporarily deployed to dvc-landing-new-docs-ho-jx8njb May 22, 2020 03:19 Inactive
@rogermparent
Copy link
Contributor Author

rogermparent commented May 22, 2020

@shcheklein I changed the Card hover animation to only background-color transitioning from white to var(--color-light-blue), which is the same as what the buttons to the right do. Unlike the small buttons, the color eases over 0.3s.

The cube.dev example has their cards start dark and brighten, while ours start bright and darken. We could invert the color, but I feel like there's too much contrast between the current two colors to do that- maybe we could use a shade of gray only barely below white, and a blue hue like the buttons' hover color to keep some consistency.

@shcheklein shcheklein temporarily deployed to dvc-landing-new-docs-ho-jx8njb May 22, 2020 04:00 Inactive
@shcheklein shcheklein temporarily deployed to dvc-landing-new-docs-ho-jx8njb May 22, 2020 04:02 Inactive
@shcheklein shcheklein temporarily deployed to dvc-landing-new-docs-ho-jx8njb May 22, 2020 04:04 Inactive
@shcheklein shcheklein temporarily deployed to dvc-landing-new-docs-ho-jx8njb May 22, 2020 05:03 Inactive
@shcheklein shcheklein temporarily deployed to dvc-landing-new-docs-ho-jx8njb May 22, 2020 05:20 Inactive
@shcheklein shcheklein temporarily deployed to dvc-landing-new-docs-ho-jx8njb May 22, 2020 05:33 Inactive
@shcheklein shcheklein temporarily deployed to dvc-landing-new-docs-ho-jx8njb May 22, 2020 05:46 Inactive
@shcheklein shcheklein merged commit 6c89356 into master May 22, 2020
@shcheklein
Copy link
Member

Done! Last step - ping designer to make it look better. Thanks @rogermparent !

@rogermparent rogermparent deleted the new-docs-home branch May 22, 2020 05:56

<card href="/doc/user-guide" heading="User Guide">

Study the detailed inner-workings of DVC in its user guide
Copy link
Contributor

Choose a reason for hiding this comment

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

Super minimal: Missing period. I'll add it no worries. Same in cmd ref card text

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants