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

feat(ui5-flexible-column-layout): new component #1867

Merged
merged 23 commits into from
Jun 30, 2020
Merged

Conversation

ilhan007
Copy link
Member

@ilhan007 ilhan007 commented Jun 24, 2020

Introduce new component in the "fiori" package, called FlexibleColumnLayout. The component implements the master-detail-detail paradigm by displaying up to three pages in separate columns. There are several possible layouts that can be changed either with the component API, or by pressing the arrows, displayed between the columns.

Fixes: #1851

Screenshot 2020-06-27 at 9 58 38 PM

ilhan007 added 6 commits June 24, 2020 13:28
The component base skeleton implemented, several layouts are supported, clicking the arrows
changes the layouts with animation
What is missing is the responsive behaviour, e.g although someone sets Three or Two column, we should display one column on small size and so on.
@ilhan007 ilhan007 changed the title POC(ui5-flexible-column-layout): new component feat(ui5-flexible-column-layout): new component Jun 24, 2020
Copy link
Contributor

@vladitasev vladitasev left a comment

Choose a reason for hiding this comment

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

  • Usually people will want to set FCL to the whole height of the page. I couldn't do this so I set height:100% to the shadow root container.
  • There are no modes for "fullscreen" so I can't try adding fullscreen buttons, especially to the third page.
  • One thing I really lack is the nav containers in the 3 columns. First thing I noticed was that it's really hard to manage the columns without creating a div and setting "slot" to it (see the dummy app). Especially if you try to use a busy indicator since it turns out the slot must be on it (it is the top-level structure in the column). You must also set height: 100% to the columns.
  • There is no easy way to change pages in the columns, you must replace the content of the whole column. So if you want to visit the same page twice, you have to cache it yourself somewhere.
  • There is no way to change pages inside one column with a transition as in OpenUI5 again due to the lack of nav container.

I currently don't think we need changes to the API of FCL, but probably it would be much easier for apps if we had a NavContainer-like control.

imagine this:

<ui5-fcl>
   <ui5-nav-container slot="beginColumn">
        <div id="page1" active>...</div>
        <div id="page2">...</div>
  </ui5-nav-container>
   </ui5-nav-container> 

Then if you want to switch to the new page, you add it to the nav container in the respecitve column and set the active property to the new page and remove it from the old page.

But again, to me this is completely separate, the FCL will not know of this nav-container-like component.

Copy link
Contributor

@vladitasev vladitasev left a comment

Choose a reason for hiding this comment

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

Some arrows seem to be missing on tablet mode (the middle size): up to 2 columns can be seen, but you always have arrows to get back to the column that is not seen.
For example go here:
https://ui5.sap.com/test-resources/sap/f/demokit/sample/ShellBarWithFlexibleColumnLayout/webapp/index.html#/detailDetail/95/1/ThreeColumnsMidExpanded
and shrink the browser a bit until the first column hides -> see the arrow to the left.

Copy link
Contributor

@vladitasev vladitasev left a comment

Choose a reason for hiding this comment

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

Almost ready, but just a couple of observations:

  • in the .hbs, the role="region" and "aria-labelledby" for the third column are probably mistakenly put on the arrow container, rather than the div holding the end column slot
  • When on "phone" mode, the column that needs to be shown is: always the third column, no matter on which 3-column layout (even on such that emphasize other columns) and the second column, no matter on which 2-column layout. Currently when you shrink, on some layouts you get the first or second column.
  • It is not possible to have two arrows pointing to the left, no matter the layout.
    image
    There should always be only one arrow per direction.
    For example, this is a layout where the third column is hidden:
    https://ui5.sap.com/test-resources/sap/f/demokit/sample/ShellBarWithFlexibleColumnLayout/webapp/index.html#/detailDetail/95/1/ThreeColumnsBeginExpandedEndHidden
    Yet, there is only one arrow, and when you click it, then you see the other arrows.

I would suggest to compare with:
https://ui5.sap.com/test-resources/sap/f/demokit/sample/ShellBarWithFlexibleColumnLayout/webapp/index.html
in order to fix the config file with the arrows and transitions.

@ilhan007 ilhan007 requested a review from vladitasev June 29, 2020 11:49
Copy link
Contributor

@vladitasev vladitasev left a comment

Choose a reason for hiding this comment

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

Works great in terms of layouts and arrow behavior!

There is only one problem I could find: when a column gets hidden as a result of hitting an arrow, the whole layout is instantly rerendered and the animation is not seen.
Example: I'm in "tablet" mode:
image
and I press the arrow to get here:
image

and there is no animation, it rerenders immediately.

What is expected: The first column should animate from 0 to 33%, the last from 66% to 0, and the second shoud just slide a bit as a side effect.

I think the solution would be to first run animations, and only then, when all columns have resized, to "hide" the extra columns. Or even not hide columns at all, just resize them to 0px, and only set a CSS to avoid the screen reader.

@ilhan007 ilhan007 requested a review from vladitasev June 30, 2020 04:12
@ilhan007 ilhan007 merged commit 7a68dd2 into master Jun 30, 2020
@ilhan007 ilhan007 deleted the new-component-fcl branch June 30, 2020 06:58
ilhan007 added a commit that referenced this pull request Jun 30, 2020
Introduce new component in the "fiori" package, called FlexibleColumnLayout. The component implements the master-detail-detail paradigm by displaying up to three pages in separate columns. There are several possible layouts that can be changed either with the component API, or by pressing the arrows, displayed between the columns.

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

Successfully merging this pull request may close these issues.

[FeatureRequest] FlexibleColumnLayout
2 participants