USACE / groundwork

React Components for USACE Applications
https://usace.github.io/groundwork
4 stars 0 forks source link

Add sidebar and breadcrumbs to Container #79

Open krowvin opened 1 week ago

krowvin commented 1 week ago

Revisions

I was uncertain if I should make a page to show how to import sidebar and then expect endusers to wrap it like you had it before in _docs-page.jsx. I ended up making it an optional part of the Container.

⚠ We may also want to consider forcing fluid to on when sidebar is present to prevent a lack of real-estate for the sidebar.

🧪 Tested this in local environment

Solves

willbreitkreutz commented 1 week ago

Hey @krowvin I've got a couple thoughts moving forward with this one.

  1. Can we separate the code highlighting from the sidebar PR? I'd prefer if we could treat them as separate issues. I'm a little hesitant on the react-syntax-highlighter lib, it adds almost 2x as many bytes as groundwork is by itself. Not that the value it adds isn't worth it, but I think it warrants a separate discussion.
  2. I'd prefer keeping the sidebar and breadcrumbs outside of the site wrapper and compose them together rather than make them options... Just my spidey senses tingling that it's backing users into a corner and will break some cases where we want sidebars on some pages, but not all etc...
krowvin commented 6 days ago

I'd prefer if we could treat them as separate issues

I agree - I should have done that. Wasn't trying to sneak it in either. But looking at the doc code had me thinking it might be nice. I'll do some more research on the syntax highligher. Surely since it's a React library some treeshaking can go down so it's not pulling in all the file types (only the ones we need) i.e. JSX, JS, and maybe bash. For now I stripped out the commits and force pushed over them as to not pollute the commit history. (assuming you don't do a squash)

Cherrypicked the commits into a sep branch for testing later: https://github.com/USACE/groundwork/tree/features/syntax-highlighting

I'd prefer keeping the sidebar and breadcrumbs outside of the site wrapper and compose them together rather than make them options

I struggled to decide on this. I was hoping to just have sidebar be a component you could use with the others when you wanted. But could not find a clean way to add the grid layout you had so the user wouldn't have to do that. But maybe that's the answer, to have that grid in the example.

I went ahead and resubmitted with Sidebar outside of the site wrapper/container.

to something like this

import { UsaceBox, Sidebar } from "@usace/groundwork";
import sidebarLinks from "./sidebarLinks";
<Container fluid>
    <div className="gw-grid gw-grid-cols-12 gw-gap-6">
        <div className="gw-hidden md:gw-block md:gw-col-span-2">
            <Sidebar sidebarLinks={sidebarLinks} />
        </div>
        <div className="gw-col-span-12 md:gw-col-span-10">
            <UsaceBox title="Sidebar Example">
                Hello World
            </UsaceBox>
        </div>
    </div>
</Container>