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

Addition of graphical layer management for canvas layouts #1187

Merged
merged 3 commits into from
Nov 7, 2021

Conversation

tarag
Copy link
Contributor

@tarag tarag commented Oct 25, 2021

Addition of layers as an intermediate component between canvas layouts and canvas items.

Allows creation of pages with various set of widgets that can easily be toggled between one another. Layer visibility management in edition improves usability when lots of widgets are visible in one canvas.

This is not backwards compatible with previous canvas, so I think it should be merged before to many people start experimenting with canvases in the milestones.

Existing canvas layouts can be easily ported by adding the following YAML lines in the code editor, between canvas and the first canvas-item:

canvas: # existing line
  - component: oh-canvas-layer
    slots:
      default:
        - component: oh-canvas-item # existing line

Preview:
Screen Recording 2021-10-25 at 18 20 10

…te component between layouts and items).

Signed-off-by: Gautier Taravella <[email protected]>
@tarag tarag requested a review from a team as a code owner October 25, 2021 16:26
@relativeci
Copy link

relativeci bot commented Oct 25, 2021

Job #235: Bundle Size — 10.66MB (~+0.01%).

b9f6789 vs edca868

Changed metrics (3/8)
Metric Current Baseline
Initial JS 1.65MB(+0.08%) 1.65MB
Cache Invalidation 18.41% 33.06%
Modules 1509(+0.07%) 1508
Changed assets by type (1/7)
            Current     Baseline
JS 8.39MB (~+0.01%) 8.39MB

View Job #235 report on app.relative-ci.com

@ghys
Copy link
Member

ghys commented Oct 25, 2021

Thanks Gautier!

I played with it a little while I had the dev server running, though initially I was confused about how to leverage these layers at runtime. Eventually I realized you meant it as a design-time-only feature, correct? There would be some missed potential if that were the case.

So after some tinkering since the layers apparently honor the visible & visibleTo properties (which would allow e.g. showing some layers to admins only), I managed to have it working at runtime. Which is great!
This could however be difficult to grasp - you have to master the visibility properties and the variables system to make it work. I think this should be either streamlined or properly documented and advertised, though for now it's fine as a first step.

2021-10-25_20-10-06

YAML
config:
  layoutType: fixed
  fixedType: canvas
  label: Canvas Layers
  activeIdx: 2
  sidebar: true
  defineVars: {}
blocks: []
masonry: null
grid: []
canvas:
  - component: oh-canvas-layer
    config:
      editVisible: false
      visible: =!vars.layer || vars.layer === 1
    slots:
      default:
        - component: oh-canvas-item
          config:
            x: 211
            y: 93
            h: 115
            w: 141
          slots:
            default:
              - component: oh-toggle-card
                config: {}
        - component: oh-canvas-item
          config:
            x: 448
            y: 156
            h: 124
            w: 150
          slots:
            default:
              - component: oh-toggle-card
                config: {}
  - component: oh-canvas-layer
    config:
      editVisible: false
      visible: =vars.layer === 2
    slots:
      default:
        - component: oh-canvas-item
          config:
            x: 241
            y: 64
            h: 150
            w: 200
          slots:
            default:
              - component: oh-knob-card
                config: {}
  - component: oh-canvas-layer
    config:
      layerName: Buttons
    slots:
      default:
        - component: oh-canvas-item
          config:
            x: 832
            y: 43
            h: 69
            w: 195
          slots:
            default:
              - component: oh-label-card
                config:
                  label: Layer 1
                  action: variable
                  actionVariable: layer
                  actionVariableValue: 1
        - component: oh-canvas-item
          config:
            x: 832
            y: 143
            h: 69
            w: 195
          slots:
            default:
              - component: oh-label-card
                config:
                  label: Layer 2
                  action: variable
                  actionVariable: layer
                  actionVariableValue: 2
        - component: oh-canvas-item
          config:
            x: 838
            y: 252
            h: 150
            w: 200
          slots:
            default:
              - component: oh-label-card
                config:
                  label: =JSON.stringify(vars.layer)

I'll review it soon, but in the meantime, I noticed some weaknesses in the implementation:

  • the layer management menu entries should not be shown if there are no layers. I noticed a first layer is automatically created when you add a widget and there are none, but still you can delete all layers either with the menu or in YAML and then things will start to break.
  • the "Show/Hide Other Layers" are rather confusing since it's reset when you switch layers.

But great work overall!

@tarag
Copy link
Contributor Author

tarag commented Oct 25, 2021

Code-wise it looks like it is a design time feature but I am using it exactly for the purpose that you found: in combination with a variable and expression set tovisibleproperty that relies on this variable. In the example I added in the PR comment each function button in the bottom toggles visibility to the corresponding layer.

I think that the vars and visible existing implementation provides a lots of flexibility for this purpose. I agree with you that is is not straightforward, I did struggle a little with the scope of variables not reaching the page as you can there: https://community.openhab.org/t/mainui-variable-not-refreshed-across-several-widgets/127794.

I will provide an example in the documentation. Having the visible property appear in the configuration panel for the layer would make sense. Is there a reason why it is not the case for widgets?

the layer management menu entries should not be shown if there are no layers. I noticed a first layer is automatically created when you add a widget and there are none, but still you can delete all layers either with the menu or in YAML and then things will start to break.

I agree with you that I can simplify the UI when there is only one layer.

the "Show/Hide Other Layers" are rather confusing since it's reset when you switch layers.

I will check what you stated about the hide/show button.
They are meant to toggle the visibility of all but the selected (active) layer. It is not the best UI but it gets the job done with minimal code. Here we are clearly only dealing with an editor feature.

…on for better performance when switching layers. Most layer menu items are no longer shown if less than 2 layers are present in the canvas.

Signed-off-by: Gautier Taravella <[email protected]>
@ghys
Copy link
Member

ghys commented Oct 30, 2021

Having the visible property appear in the configuration panel for the layer would make sense. Is there a reason why it is not the case for widgets?

No reason, it's somewhat in the backlog but could possibly be added for all widgets that support it (as an advanced property group), as well as other common properties like visibleTo, class or style (the latter giving you an editor tailored for CSS with autocompletion, perhaps).

@tarag
Copy link
Contributor Author

tarag commented Nov 6, 2021

@ghys , do you thing we could get this merged before the imminent next milestone, so that people experimenting the canvas layout would not have to migrate their layout ?

@relativeci
Copy link

relativeci bot commented Nov 7, 2021

Job #251: Bundle Size — 10.75MB (+0.06%).

2355c31 vs 2ef2225

Changed metrics (4/8)
Metric Current Baseline
Initial JS 1.66MB(+0.39%) 1.65MB
Initial CSS 605.05KB(+0.02%) 604.96KB
Cache Invalidation 25.42% 0.57%
Modules 1529(+0.2%) 1526
Changed assets by type (2/7)
            Current     Baseline
CSS 848.96KB (+0.01%) 848.87KB
JS 8.47MB (+0.07%) 8.47MB

View Job #251 report on app.relative-ci.com

Copy link
Member

@ghys ghys left a comment

Choose a reason for hiding this comment

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

@tarag I'd say it looks pretty good now.
I fixed a minor bug which broke the component docs generation script, which I run. It's a manual process for now, so please remember to give it a run when you add/change widget parameter definitions.

Also as openhab/openhab-docs#1657 is still open, perhaps you could describe the layers feature there as well while you're at it?

Thanks!

@ghys ghys added enhancement New feature or request main ui Main UI labels Nov 7, 2021
@ghys ghys added this to the 3.2 milestone Nov 7, 2021
@ghys ghys merged commit 9aeb301 into openhab:main Nov 7, 2021
@tarag
Copy link
Contributor Author

tarag commented Nov 8, 2021

Thanks, I'll try to think about it for the parameter docs.

I'll update the docs for the layers feature soon.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request main ui Main UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants