liferay / clay

A web implementation of the Lexicon Experience Language
http://clayui.com
Other
206 stars 470 forks source link

Vertical Bar Component #2903

Closed jbalsas closed 2 years ago

jbalsas commented 4 years ago

This is a formal request to implement Application Sidebar as a Clay Component.

We currently lack documentation proper Lexicon definition and documentation for this component, so this task should start by involving @drakonux and @laugardie in analysis and definition tasks. This is already requested in LEXI-806

 Current Existing Implementations:

PageEditor Sidebar
Screen Shot 2020-02-05 at 12 58 25

Future Implementations:

Product Menu

We currently have a POC for this at https://github.com/eduardoallegrini/liferay-product-menu

Screen Shot 2020-02-05 at 13 59 37
Journal Sidebar

image (3)

/cc @bryceosterhaus, @kresimir-coko, feel free to ping @drakonux for any clarification

bryceosterhaus commented 4 years ago

@jbalsas is this already an existing component via clay-css?

jbalsas commented 4 years ago

I don't think so, but @pat270 might know best. I think @eduardoallegrini and @marcoscv-work can work on this with @pat270 or he can use the linked POC as a base.

pat270 commented 4 years ago

@jbalsas @bryceosterhaus @marcoscv-work @eduardoallegrini We will need to add a base component for the skinny toolbar with icons. I'm thinking .tbar-stacked? We will also need to create variations for each, .tbar-product-menu and .tbar-journal-sidebar?

The Product Menu and Journal Sidebar can be Sidebar variations. I think we should name them .sidebar-product-menu and .sidebar-journal.

For the search bar in Product Menu we will need details about focus, disabled and placeholder styles from @drakonux and @laugardie.

Feel free to send a PR otherwise I can get to it after I work on the other milestone 15 stuff.

diegonvs commented 4 years ago

We will also need to create variations for each, .tbar-product-menu and .tbar-journal-sidebar?

I think these sidebar looks the same, only differs by the content. May We could create dark and light variations.

edalgrin commented 4 years ago

Can't we use general names instead of dedicated one? E.g. .tbar-left, .tbar-previous, .tbar-before in this way we can use the same tbars without worry about the misunderstandings the names could create. The same goes for the sidebars .sidebar-left etc..

jbalsas commented 4 years ago

Yeah, these are not application specific. We're likely to have:

But they can be used in different apps other than journal and product menu, so we should make them as generic as possible

edalgrin commented 4 years ago

I'm going to work on it

pat270 commented 4 years ago

Going forward, I'm trying to address some of the concerns expressed in https://github.com/liferay/clay/issues/1104. We should refrain from being too generic because modifying sidebar-dark would cause changes on admin related UI. It seems that is an unwanted behavior for a lot of folks. We really should free up the generic namespaces for devs and scope our stuff to something a little more specific. The component name can be generic relative to Liferay Portal Admin in my opinion.

diegonvs commented 4 years ago

Hey folks, considering that inside an sidebar we can use collapses or anything I created this draft:

<ClaySidebar displayType="dark" position="left">
    <ClaySidebar.Buttons buttons={buttons} diplayType="tabs?" />
    <ClaySidebar.Contents>
        <WidgetsSidebarContent id="widgets" label="Widgets"/>
        <ApplicationsSidebarContent id="apps" label="Applications"/>
        <FragmentsSidebarContent id="apps" label="Applications"/>
    </ClaySidebar.Contents>
</ClaySidebar>

const buttons = 
[
    {
        icon: 'square',
        id: 'widgets',
        renderIcon?: (children, openCallback) => (<div className="bg-success" onClick={openCallback}> {children} </div>)
    },
    {
        icon: 'app',
        id: 'apps',
        label: 'Applications',
        renderIcon?: (children, openCallback) => (<div className="bg-success" onClick={openCallback}> {children} </div>)
    },
    {
        separator
    },
    {
        icon: 'fragments',
        id: 'fragments',
        label: 'Fragments',
        renderIcon?: (children, openCallback) => (<div className="bg-success" onClick={openCallback}> {children} </div>)
    },
]

Where buttons.id can be used to display the sidebar content.

What are you thoughts on this?

bryceosterhaus commented 4 years ago

@diegonvs how do you see the id being used to display/hide the content? Generally I think we should avoid ids as a required prop since they must be unique on the page.

diegonvs commented 4 years ago

@diegonvs how do you see the id being used to display/hide the content?

It just tells to the ClaySidebar.Contents component: "- Hey, the user selected this button, so, render me a sidebar body with this component."

The logic to show and hide the Sidebar will be handled by ClaySidebar component.

Generally I think we should avoid ids as a required prop since they must be unique on the page.

We could rename this property.

jbalsas commented 4 years ago

Going forward, I'm trying to address some of the concerns expressed in #1104. We should refrain from being too generic because modifying sidebar-dark would cause changes on admin related UI. It seems that is an unwanted behaviour for a lot of folks. We really should free up the generic namespaces for devs and scope our stuff to something a little more specific. The component name can be generic relative to Liferay Portal Admin in my opinion.

Hey @pat270, I think this is both the beauty and the curse of CSS, isn't it? :)

In general, we need to carefully analyze such reports so we don't put the bandaid where it's not needed. There's a lot of things to be considered and same way people expect some things not to change, other people expect the exact opposite.

In this case, as we move to SaaS, it's critical that we have a coherent set of components that all react cohesively to a small set of variations (skins). This is becoming increasingly important to our company as we try to make the jump to SaaS offerings like Commerce.

With all that in mind, as I recognize this is a real issue and other solutions might be harder to implement (shadow DOM) or more harmful to users (documentation), I think we could move with the .c- prefix for our components... but nothing with the -journal or -product-menu suffixes :)

pat270 commented 4 years ago

@jbalsas .c- prefix is good, anything that will differentiate those. I still think we need specific classes for each bar. Clay doesn't need to add them, but whoever is making those specific tbar and sidebar should include them.

jbalsas commented 4 years ago

Removing from the milestone for now, since this will likely take additional time

pat270 commented 3 years ago

Is this one still valid? The component teams have already created their sidebars in DXP. Product Menu sidebar was replaced with Application Menu. Feel free to reopen if it is.

drakonux commented 3 years ago

We created the vertical bar time ago to solve all the cases in DXP.

Is still valid? I would say yes, it's still valid and we should create the component and replace all the bars in DXP with a vertical bar component (customized in some cases). But I'm not the owner of DXP... nor do I know the plans of FI to regularize its use... so I summon you @nhpatt @dsanz

drakonux commented 3 years ago

In fact, Corbin asked for this time ago see #3959

pat270 commented 3 years ago

@drakonux I will reopen

javiergamarra commented 3 years ago

Yes, this is still valid as there is a definition in lexicon that is not implemented in Clay (just one, tables and treeview does not exist :P)

github-actions[bot] commented 2 years ago

This issue has been merged and will be released in DXP at https://issues.liferay.com/browse/LPS-155343