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

Visual Layout Editor #1369

Merged
merged 49 commits into from
Nov 1, 2024
Merged

Visual Layout Editor #1369

merged 49 commits into from
Nov 1, 2024

Conversation

joepavitt
Copy link
Collaborator

@joepavitt joepavitt commented Oct 8, 2024

Description

Making this Draft PR for handover with Steve. detailing the todo list so it makes it clear what steps are required to completion and release readiness.

Done So Far

  • Interactive elements in UI to re-order elements
  • Handlers added to resize groups

Todo List

Todo

Preview Give feedback

Scope

  • Limited to group re-ordering and resizing

Related Issue(s)

Closes #30

@joepavitt joepavitt changed the title 30 wysiwyg editor Visual Layout Editor Oct 8, 2024
@Steve-Mcl Steve-Mcl self-assigned this Oct 8, 2024
@joepavitt
Copy link
Collaborator Author

Could you do a quick share of when you have 5-6 elements in the page too please?

@Steve-Mcl
Copy link
Contributor

Could you do a quick share of when you have 5-6 elements in the page too please?

chrome_XKo1mynU1d

NOTE: I can revert to the "immediate" update but we might have to handle the drag+drop manually due to drop effect cursor limitations.

@joepavitt
Copy link
Collaborator Author

I'm struggling with this, it's not clear to me where the element is going to drop into based on the hovering you're showing unfortunately.

@Steve-Mcl
Copy link
Contributor

I'm struggling with this, it's not clear to me where the element is going to drop into based on the hovering you're showing unfortunately.

When i come back onto this PR, i will revisit. Perhaps I missed something - but I did spent a lot of time in the docs and examples - with numerous workarounds and hacks - I got nowhere. Simply doing the drag drop in a more standard way (without mutating the groups below the cursor while moving) things simplified greatly and the drop effects started to work.

@joepavitt
Copy link
Collaborator Author

Simply doing the drag drop in a more standard way

That isn't a standard way though. Node-RED's editable list works how I had built it:

Screen.Recording.2024-10-16.at.09.32.01.mov

@Steve-Mcl
Copy link
Contributor

Simply doing the drag drop in a more standard way

That isn't a standard way though. Node-RED's editable list works how I had built it:

Screen.Recording.2024-10-16.at.09.32.01.mov

Node-RED list does not mutate the underlying structure until the item is dropped - it uses a dummy placeholder to simulate the placement. Additionally, all elements in a regular list are the same size. If they were not, Node-RED would have similar considerations to make regarding flip-flop of elements. It also doesnt use the new dataTransfer drag/drop mechanism (uses older tech)

@Steve-Mcl
Copy link
Contributor

Not directly related to this work (though it is possible) is that when edits are deployed, ui-config is transmitted to clients - and I see them blink momentarily - they do not update to new layout.

Hmm, this is a bug. I assume you have two instances of the Dashboard open here? The new ui-config should contain the updated width values.

I think you encountered this in your tutorial video - agree it is a bug unrelated to this PR. I think there is an existing issue - will follow up shortly.

@Steve-Mcl
Copy link
Contributor

RE commit:
Revert drop on move, fixes drag cursor, improves sizing accuracy + re…

Reverted the drag drop to "place on drag-over" as before BUT now working with dropEffect cursor

chrome_DAF5RZjI6N

improves sizing accuracy

This took far more experimentation and brain power than it should have.
In the end, it was as simple as using the actual parent v-cards width for calculating divisions AND using the newly calculated width to determine if a size increase or size decrease is permitted based on being within 85% of the next step. Now resizing happens close to the snap point, doesn't weirdly snap at different points across the range, step size is consistently applied across the full length of the operation. Phew!

chrome_ZAxYxaruxX

Refactoring

  • main mixin index.js loads resizable.js mixin and movable.js mixin
  • styles for resizable are in an importable scss file
  • much of the extra boilerplace code has been removed from the group.vue and grid/flex vue files

There are still some possible refactoring's and they will probably become more evident with the feature set grows - for now, it is in much nicer shape (IMO)

@Steve-Mcl
Copy link
Contributor

Found issues while testing on staging (potential blockers)

  • When a group contains no widgets, doing an admin API deploy (like we do) causes ui_base.js::node.deregister. Effectively the page is removed from ui.pages map. This makes subsequent wysywig edits impossible until the page is edited and deployed within Node-RED

    • We may need to look at the registration/deregistration methods in DB2 and how/when they are called
  • Not directly related to this work (though it is possible) is that when edits are deployed, ui-config is transmitted to clients - and I see them blink momentarily - they do not update to new layout.

    • The WS message for the edited group seems to still have the old width setting
    • Refreshing the page delivers the new config correctly.

These issues are still TODO

@Steve-Mcl
Copy link
Contributor

Steve-Mcl commented Oct 28, 2024

@joepavitt fixed remaining issues.

Remaining tasks are:

  • Dedicate E2E Tests
  • Docs for wysiwyg

How do we want to proceed - get this in early (already pretty large PR) - or - keep going to get tests and docs as part of this PR?


PS: Regarding the task you self assigned:

Clean the iconography & colours in the "Action" bar. @joepavitt to resolve this

  • Do you want me to convert that to an issue so you can branch off this branch?
  • Or do you want to provide colors & I'll update this branch?
  • Or are you going to pull, edit, push this branch?

@Steve-Mcl Steve-Mcl marked this pull request as ready for review October 29, 2024 16:23
@Steve-Mcl Steve-Mcl mentioned this pull request Oct 31, 2024
12 tasks
@joepavitt
Copy link
Collaborator Author

Review:

  • With httpAdminRoot set to something other than default, the "Edit Layout" button returns a 404
  • Having issues rendering a fair few pages, just seen blank content. Can't work out why, needs further investigation

@joepavitt
Copy link
Collaborator Author

joepavitt commented Nov 1, 2024

Just pushed fix for the broken E2E, but otherwise this is looking excellent!

I can't approve/merge from my side as I opened the original PR, but you have my approval @Steve-Mcl

@joepavitt joepavitt requested a review from Steve-Mcl November 1, 2024 18:46
@joepavitt
Copy link
Collaborator Author

I'll then do an icon clean up as a follow on piece once merged

@Steve-Mcl Steve-Mcl merged commit 7df37fe into main Nov 1, 2024
1 of 2 checks passed
@Steve-Mcl Steve-Mcl deleted the 30-wysiwyg-editor branch November 1, 2024 21:55
@Steve-Mcl Steve-Mcl mentioned this pull request Nov 14, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Visual Layout Editor
2 participants