homebrewery
homebrewery copied to clipboard
Unify base page structure - ListPage
#1423 was an attempt to unify the underlying editor page for NewPage
and EditPage
. However the rest of the project moved on before the PR was completed.
This issue is to record the idea - unifying the underlying page structure so that functions do not need to be modified in multiple places - so that it is not lost with the closure of #1423.
My intention is to close #1423 and open a new PR with a smaller, sleeker set of changes, implementing a minimal combined EditorPage to be called by NewPage and EditPage, which can be merged further through later PRs. I also intend to push UserPage through an underlying ListPage, which can be used for any future pages that need to list multiple brews.
This issue is to track the creation of the ListPage base page.
Updated the Issue description to reflect the separation of ListPage and EditorPage base pages.
I'm going to reopen this Issue, for now, but maybe a new issue is better. I'm happy to go with whichever direction makes most sense....
I don't think this is issue is totally resolved, if taking it's title to the maximum extent: All pages should have one base page upon which their specific functions are built. It seems like right now we only have the "brew list" and "account page" set to use one common base...but nothing else is on that common base. Even if "new" and "edit" and "share" and maybe others have slightly different sets of buttons in their navbars, they can still rest on that same basic layout, rather than each laying out their navbars separately.
A minor suggestion is to rename the uiPage.jsx
to baseLayout.jsx
or something similarly descriptive of the function of that component. I think uiPage.jsx
falls short in describing the purpose of that file (everything is "UI", and it's not really a "Page" either).
Look at #1800 which is the sub-issue specifically for unifying EditPage, NewPage, and HomePage, and the accompanying PR #1801 which I would love to see restarted and completed.
This issue specifically targets the "listpage" base, which is complete. I would make a new issue for any remaining "base pages" that don't already have their own issue.
This is sort of an incomplete thought but I have to go to bed and likely toy with the code a bit later. I'm just hitting submit so I can come back to it later.
Is this really complete though? I see this structure right now:
userPage.jsx
--> listPage.jsx
userPage
has it's own navbar render method which it then passes to listPage
. userPage
is basically doing the same thing as what uiPage.jsx
does, and I think we can unify what that "top level" component is since both userPage
and uiPage
are basically containers for Navbar + Page.
Right now I envision these changes:
- Rename
uiPage.jsx
tobaseLayout.jsx
--- this holds the Navbar and the container for "page content" - More-or-less replace everything in
userPage.jsx
with the contents oflistPage.jsx
--- "listPage" doesn't mean much, or doesn't have the same level of meaning as "user page". If for some reason we want to re-use the "listing" component, we can break that out as well into a more generic component, but I don't think this is likely for some time. - get rid of
listPage.jsx
So the result is uiPage/baseLayout.jsx
--> userPage.jsx
.
This would be similar to uiPage/baseLayout.jsx
--> accountPage.jsx
, and could be the pattern for Edit, Share, etc. Obviously each of those is going to have different buttons in the navbar (each of which is it's own component), which I think we can designate at the higher level homebrew.jsx
file, passing a list of nav buttons needed as a prop. Not 100% sure that is the only/best way though.
Something approximately like this, just using nav components that are handy and not necessarily the ones we want:
// baseLayout.jsx
const BaseLayout = createClass({
displayName : 'BaseLayout',
render : function(){
return <div className='baseLayout sitePage'>
<Navbar>
<Nav.section>
<Nav.item className='brewTitle'>{this.props.brew.title}</Nav.item>
</Nav.section>
<Nav.section>
{this.props.navItems} // custom page-specific navItems inserted in front of every-page navItems
<NewBrewItem />
<HelpNavItem />
<RecentNavItem />
<Account />
</Nav.section>
</Navbar>
<div className='content'>
{this.props.pageContent}
</div>
</div>;
}
});
// accountPage.jsx
...
render : function(){
return <BaseLayout
brew={this.props.brew}
pageContent={this.renderUiItems()}
navItems={
<>
<PrintItem />
<RedditItem />
</>}>
</BaseLayout>;
}
...