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

Library layout components, simplified #144

Merged
merged 2 commits into from
Jul 15, 2021
Merged

Conversation

lyzadanger
Copy link
Contributor

@lyzadanger lyzadanger commented Jul 14, 2021

This PR proposes a new collection of rather simple components for rendering Pattern Library pages: Page, Pattern, Example and Demo. These components represent an improvement on existing pattern-library components:

  • The structure is simpler and more obvious
  • The layout is responsive
  • The styling is simpler, has no leakage into patterns, and uses and follows some existing underlying patterns
  • Source JSX for demos is optional, and is put into a separate tab so as not to clutter the interface as much

This PR contains a component module with these components (Library), styles, and an example of one of the pattern-library pages converted to use the new components.

This represents a coalescing of “lessons learned” from the first crop of pattern-library components. This was a quick implementation based on the growing set of underlying tools and patterns!

Before

"Container components" page before these changes:

Screen Shot 2021-07-14 at 10 15 57 AM

Layout breaks on narrow screens:

Screen Shot 2021-07-14 at 10 17 11 AM

After

Screen Shot 2021-07-14 at 10 14 05 AM

The layout is now responsive:

Screen Shot 2021-07-14 at 10 15 28 AM

Proposed Model

In this model, a Page contains zero or more Pattern sections. A Pattern contains zero or more Examples. And an Example contains zero or more Demos.

Demo takes a withSource boolean prop. When true, it will render into a tabbed format. Here's what it looks like with the "Source" tab activated:

Screen Shot 2021-07-14 at 10 14 31 AM

When false, it will not provide source (second "Demo" here does not have source):

Screen Shot 2021-07-14 at 10 15 02 AM

All of these components are in a single module Library. Because the expectation is to import the set of components at once (e.g. import * as Library from ‘../Library’;) I’ve elected to leave the module name upper-case. Our convention for component modules that export multiple components has been to this point lower-case, however, this usage model is different. Discussion welcomed!

Previous Model and shortcomings

(For reference if interested).

The previous model is PatternPage, Pattern, PatternExamples, PatternExample.

  • PatternExamples uses a tabular layout, which is most often fine, but can cumbersome to manage in layout. And it’s not responsive.
  • Some of the styling uses element selectors and/or relies on base selector styles. This can cause leakage into rendered patterns, and other complexity. The styling is also rather verbose and ad-hoc as it has developed over time.
  • PatternExample will always render its source, which is sometimes not what you’d want. The “middle column” of an example row is kind of useless at present. Authors are forced to put example explanations into a title prop, which is awkward.
  • PatternExamples doesn’t have much “meaning” as a component, and is used primarily to render a wrapping table.

Where to?

Assuming this kind of approach is reasonable, next steps would be to:

  • Convert other pattern-library pages to use the Library components
  • Make a package release providing access to the new Library components
  • Update any third-party pattern-library pages (known one in the client, will have to verify that LMS does not have any) to use Library components
  • Remove excess styling and unused PatternPage components

Fixes #102

@esanzgar esanzgar self-requested a review July 14, 2021 14:56
Copy link
Contributor

@esanzgar esanzgar left a comment

Choose a reason for hiding this comment

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

I would suggest to export the components from Library.js in this way:

export default {
  Page,
  Pattern,
  Example,
  Demo,
};

So it can be imported without using the asterisk:
`import Library from './Library``

On a personal note, I prefer UIs with less borders and backgrounds, like this:

image

$-library-vertical-spacing: 7;
.LibraryPage {
// Make sure the content width is not too terribly wide: it gets hard to read
max-width: 75rem;
Copy link
Contributor

Choose a reason for hiding this comment

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

👍🏼

@lyzadanger
Copy link
Contributor Author

@esanzgar

On a personal note, I prefer UIs with less borders and backgrounds, like this:

I think that's helpful feedback. This particular page of patterns is a little hard to assess regarding "frames" because several of the components are frames themselves. Which leads to a sense of overwhelming frame-i-ness. If I might ask permission to take a look at this during the conversion of the next set of pages to this model? That way I can see how "demo" layout feels in real-world demo use cases and try to see if I can find a simpler style.

@lyzadanger lyzadanger force-pushed the simplified-library-layout branch from 4698e1e to f0ef4b4 Compare July 15, 2021 13:14
@lyzadanger lyzadanger merged commit cdb3a17 into main Jul 15, 2021
@lyzadanger lyzadanger deleted the simplified-library-layout branch July 15, 2021 13:15
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.

Clean up Pattern Library components and styling
2 participants