Add: New group display type of Dialog #1364

gayanSandamal · 2024-10-07T06:06:00Z

Description

Note

Note that sometimes the E2E test of control.spec.js fails randomly

Related Issue(s)

#1106

Checklist

I have read the contribution guidelines
Suitable unit/system level tests have been added and they pass
Documentation has been updated
- Upgrade instructions
- Configuration details
- Concepts
Changes flowforge.yml?
- Issue/PR raised on FlowFuse/helm to update ConfigMap Template
- Issue/PR raised on FlowFuse/CloudProject to update values for Staging/Production

Labels

Includes a DB migration? -> add the area:migration label

joepavitt · 2024-10-07T09:35:51Z

Note that sometimes the E2E test of control.spec.js fails randomly

As in, the new test you've added is randomly failing?

gayanSandamal · 2024-10-07T10:57:18Z

Note that sometimes the E2E test of control.spec.js fails randomly

As in, the new test you've added is randomly failing?

Not only the new test, the control.spec.js was failing randomly for months

joepavitt · 2024-10-07T11:03:41Z

I know that, but i mean is your new test now included in part of the randomly failings tests, or is your new test passing consistently?

gayanSandamal · 2024-10-07T11:14:18Z

I know that, but i mean is your new test now included in part of the randomly failings tests, or is your new test passing consistently?

Yes, the new test is a part of the randomly failing tests

gayanSandamal · 2024-10-07T11:20:10Z

I know that, but i mean is your new test now included in part of the randomly failings tests, or is your new test passing consistently?

@joepavitt I can remove the newly added test as they are randomly failing

joepavitt

Configuration

It doesn't feel logical that the type row would live underneath the "Default State" options. I think it should be between the "Page" and "Size" rows.

Sidebar Icon

Not essential, but it'd be nice to use an alternative icon here in the sidebar configuration for "Dialog" type groups. Not sure what icon would make sense though. Maybe https://fontawesome.com/v4/icon/window-restore?

`show-dialog` in UI Control

Functionally, this works great, but why does this need an entirely new event type? Why not drive this from the existing show/hide logic for a group? You can still rely on the showDialog variable in the client-side, but the user-facing API (i.e. show or hide) should be ideally be consistent

gayanSandamal · 2024-10-07T21:30:39Z

@joepavitt I’ve addressed the above feedback and removed the recently added E2E tests as they were failing randomly.
This is ready for review

Configuration

It doesn't feel logical that the type row would live underneath the "Default State" options. I think it should be between the "Page" and "Size" rows.

Sidebar Icon

Not essential, but it'd be nice to use an alternative icon here in the sidebar configuration for "Dialog" type groups. Not sure what icon would make sense though. Maybe https://fontawesome.com/v4/icon/window-restore?

show-dialog in UI Control

Functionally, this works great, but why does this need an entirely new event type? Why not drive this from the existing show/hide logic for a group? You can still rely on the showDialog variable in the client-side, but the user-facing API (i.e. show or hide) should be ideally be consistent

Handled with this fix

joepavitt · 2024-10-08T11:12:26Z

Functionally - all good

Can you add documentation to the /nodes.config/ui-group page please for "Type" as a section. Then detail an screenshot example of a normal group, and another with a Dialog example.

This flow will also be a useful example to include:

Dialog Groups Example:

[
  {
      "id": "0aa38a714b8a3d67",
      "type": "ui-form",
      "z": "9337c17e1a7f6875",
      "name": "",
      "group": "c7871ac53089d535",
      "label": "New User",
      "order": 1,
      "width": 0,
      "height": 0,
      "options": [
          {
              "label": "Name",
              "key": "name",
              "type": "text",
              "required": true,
              "rows": null
          },
          {
              "label": "Admin",
              "key": "isAdmin",
              "type": "switch",
              "required": false,
              "rows": null
          }
      ],
      "formValue": {
          "name": "",
          "isAdmin": false
      },
      "payload": "",
      "submit": "Add",
      "cancel": "clear",
      "resetOnSubmit": true,
      "topic": "topic",
      "topicType": "msg",
      "splitLayout": "",
      "className": "",
      "passthru": false,
      "dropdownOptions": [],
      "x": 220,
      "y": 100,
      "wires": [
          [
              "adbd76ecf97076a1"
          ]
      ]
  },
  {
      "id": "fa4925a5253341ce",
      "type": "ui-control",
      "z": "9337c17e1a7f6875",
      "name": "",
      "ui": "c2e1aa56f50f03bd",
      "events": "all",
      "x": 580,
      "y": 100,
      "wires": [
          []
      ]
  },
  {
      "id": "adbd76ecf97076a1",
      "type": "change",
      "z": "9337c17e1a7f6875",
      "name": "Hide Dialog",
      "rules": [
          {
              "t": "set",
              "p": "payload",
              "pt": "msg",
              "to": "{\"groups\":{\"hide\":[\"Active Development:Dialog Group\"]}}",
              "tot": "json"
          }
      ],
      "action": "",
      "property": "",
      "from": "",
      "to": "",
      "reg": false,
      "x": 390,
      "y": 100,
      "wires": [
          [
              "fa4925a5253341ce",
              "b9d77a856b1be020"
          ]
      ]
  },
  {
      "id": "294ac777d99f5789",
      "type": "ui-button",
      "z": "9337c17e1a7f6875",
      "group": "497106faf38a5190",
      "name": "",
      "label": "Add User (Dialog)",
      "order": 1,
      "width": 0,
      "height": 0,
      "emulateClick": false,
      "tooltip": "",
      "color": "",
      "bgcolor": "",
      "className": "",
      "icon": "",
      "iconPosition": "left",
      "payload": "{\"groups\":{\"show\":[\"Active Development:Dialog Group\"]}}",
      "payloadType": "json",
      "topic": "topic",
      "topicType": "msg",
      "buttonColor": "",
      "textColor": "",
      "iconColor": "",
      "x": 370,
      "y": 140,
      "wires": [
          [
              "fa4925a5253341ce",
              "b9d77a856b1be020"
          ]
      ]
  },
  {
      "id": "b9d77a856b1be020",
      "type": "debug",
      "z": "9337c17e1a7f6875",
      "name": "debug 2572",
      "active": true,
      "tosidebar": true,
      "console": false,
      "tostatus": false,
      "complete": "false",
      "statusVal": "",
      "statusType": "auto",
      "x": 590,
      "y": 140,
      "wires": []
  },
  {
      "id": "c7871ac53089d535",
      "type": "ui-group",
      "name": "Dialog Group",
      "page": "22b5519aa675ad88",
      "width": "6",
      "height": "1",
      "order": 1,
      "showTitle": true,
      "className": "",
      "visible": "true",
      "disabled": "false",
      "groupType": "dialog"
  },
  {
      "id": "c2e1aa56f50f03bd",
      "type": "ui-base",
      "name": "Dashboard",
      "path": "/dashboard",
      "includeClientData": true,
      "acceptsClientConfig": [
          "ui-control",
          "ui-notification"
      ],
      "showPathInSidebar": false,
      "showPageTitle": false,
      "navigationStyle": "icon",
      "titleBarStyle": "default"
  },
  {
      "id": "497106faf38a5190",
      "type": "ui-group",
      "name": "Button Groups",
      "page": "22b5519aa675ad88",
      "width": "6",
      "height": "1",
      "order": 10,
      "showTitle": true,
      "className": "",
      "visible": "true",
      "disabled": "false"
  },
  {
      "id": "22b5519aa675ad88",
      "type": "ui-page",
      "name": "Active Development",
      "ui": "c2e1aa56f50f03bd",
      "path": "/active-development",
      "icon": "forum",
      "layout": "grid",
      "theme": "129e99574def90a3",
      "order": 1,
      "className": "",
      "visible": "true",
      "disabled": "false"
  },
  {
      "id": "129e99574def90a3",
      "type": "ui-theme",
      "name": "Custom Theme",
      "colors": {
          "surface": "#000000",
          "primary": "#ff4000",
          "bgPage": "#f0f0f0",
          "groupBg": "#ffffff",
          "groupOutline": "#d9d9d9"
      },
      "sizes": {
          "pagePadding": "24px",
          "groupGap": "12px",
          "groupBorderRadius": "9px",
          "widgetGap": "6px",
          "density": "default"
      }
  }
]

With that documentation, also mention how you need to use a ui-control to show/hide the group, and link to the relevant documentation

gayanSandamal · 2024-10-08T11:15:58Z

Functionally - all good

Can you add documentation to the /nodes.config/ui-group page please for "Type" as a section. Then detail an screenshot example of a normal group, and another with a Dialog example.

This flow will also be a useful example to include:

Dialog Groups Example:
With that documentation, also mention how you need to use a ui-control to show/hide the group, and link to the relevant documentation

Sure, I will update

joepavitt · 2024-10-08T11:18:50Z

The only thing we also need to look at is the Dialog width. You have a hardcoded max-width of 500px? We should be smarter than that. Maybe the width of the screen * group width (columns) / 12?

Similar to how the grid layout works, still gives the user control. Although we need to consider the relevant breakpoints too, whereby it'd just render full-width anyway.

gayanSandamal · 2024-10-08T11:30:44Z

The only thing we also need to look at is the Dialog width. You have a hardcoded max-width of 500px? We should be smarter than that. Maybe the width of the screen * group width (columns) / 12?

Similar to how the grid layout works, still gives the user control. Although we need to consider the relevant breakpoints too, whereby it'd just render full-width anyway.

Noted, I will update the code with that.
As per the position of the container of the Dialog is fixed. We may not be able to use CSS Grid. If so, I will use the 12 column grid behaviour to set the width based on the percentage calculated by the given NR columns.
Also we need to ignore the number of rows and make the dialog scrollable based on its content inside

joepavitt · 2024-10-08T11:48:21Z

If so, I will use the 12 column grid behaviour to set the width based on the percentage calculated by the given NR columns.

No need for grid CSS, just use the maths of screen width x columns / 12

joepavitt · 2024-10-08T11:48:38Z

Also we need to ignore the number of rows and make the dialog scrollable based on its content inside

Yep, I agree with that approach

gayanSandamal · 2024-10-08T11:55:15Z

If so, I will use the 12 column grid behaviour to set the width based on the percentage calculated by the given NR columns.

No need for grid CSS, just use the maths of screen width x columns / 12

Also we need to ignore the number of rows and make the dialog scrollable based on its content inside

Yep, I agree with that approach

I'm working on it

gayanSandamal · 2024-10-09T06:13:45Z

@joepavitt Addressed feedback as below

Updated docs as suggested
Added custom width support to the dialog groups
Added breakpoint for both mobile and tablet screen sizes

joepavitt · 2024-10-09T15:25:41Z

Please checkout 72dc19f for how you can render a flow viewer inline in the documentation, no need for hardcoded inline json and images.

I've pushed that for now, but just an FYI for future docs that you write.

joepavitt

Subject to passing tests, this is good to go.

gayanSandamal · 2024-10-09T17:50:20Z

Please checkout 72dc19f for how you can render a flow viewer inline in the documentation, no need for hardcoded inline json and images.

I've pushed that for now, but just an FYI for future docs that you write.

Thanks @joepavitt I will check this and note this for future docs writings

gayanSandamal · 2024-10-09T17:51:50Z

Subject to passing tests, this is good to go.

Merging as all the tests are passing

gayanSandamal added 2 commits October 7, 2024 11:31

Implementation of dialog groups for all the layout types

2eff121

Fixes

a373f27

gayanSandamal linked an issue Oct 7, 2024 that may be closed by this pull request

Have "Dialog" group types #1106

Closed

gayanSandamal added 2 commits October 7, 2024 11:43

test update

fe57f8a

increased waiting time of show dialog

8635a60

gayanSandamal requested a review from joepavitt October 7, 2024 06:55

gayanSandamal self-assigned this Oct 7, 2024

joepavitt requested changes Oct 7, 2024

View reviewed changes

gayanSandamal added 5 commits October 8, 2024 00:08

PR review fixes

1eaca7d

Doc update

6f770aa

lint fixes

cf5a8b2

UIControl show dialog error handling

bf3e847

bug fix

6146f8a

gayanSandamal requested a review from joepavitt October 7, 2024 21:30

gayanSandamal added 2 commits October 9, 2024 10:52

Docs update and NR grid width

f918282

Added breakpoints for dialog gorup width

0abc573

Use FlowViewer, not hardcoded screenshot and inline JSON

72dc19f

joepavitt added 2 commits October 9, 2024 16:27

Tidy some docs grammar

9f55d17

Minor in-editor help improvement

4a9749d

joepavitt approved these changes Oct 9, 2024

View reviewed changes

gayanSandamal merged commit 81a2150 into main Oct 9, 2024
2 checks passed

gayanSandamal deleted the 1106-have-dialog-group-types branch October 9, 2024 17:52

gayanSandamal changed the title ~~Implementation of dialog groups for all the layout types~~ Add: New group display type of Dialog Oct 10, 2024

gayanSandamal mentioned this pull request Oct 10, 2024

Ensure groupType has its default value #1378

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add: New group display type of Dialog #1364

Add: New group display type of Dialog #1364

gayanSandamal commented Oct 7, 2024 •

edited

Loading

joepavitt commented Oct 7, 2024

gayanSandamal commented Oct 7, 2024

joepavitt commented Oct 7, 2024 •

edited

Loading

gayanSandamal commented Oct 7, 2024

gayanSandamal commented Oct 7, 2024

joepavitt left a comment