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

fix: handle adding/removing tiles (widgets) #1944

Merged
merged 22 commits into from
Feb 13, 2025
Merged

Conversation

filipgutica
Copy link
Contributor

@filipgutica filipgutica commented Feb 11, 2025

Summary

https://konghq.atlassian.net/browse/MA-3668
https://konghq.atlassian.net/browse/MA-3622
https://konghq.atlassian.net/browse/MA-3621

  • Tiles passed into the gridstack grid layout component inform the initial state of the dashboard.
  • Use a copy of the passed in tiles in order to prevent circular event loops
  • Gridstack responsible for all things related to position and dimensions of tiles. On the appropriate gridstack events, regenerate tile definitions with updated layout and emit them so the host app can update the dashboard definition
  • Watch for changes to provided tile's length to handle adding new tiles. Wait for the dom to be updated await nextTick() then use makeWidget to make the newly added tile into a gridstack widget.
  • Watch for changes to the provided tile's metadata and update the appropriate tile (to handle changes to content) eg query, chart title, chart type etc
  • Use a single can-edit prop
  • Add new sandbox page for editable dashboard, remove changes to "static" dashboard sandbox
  • Track unique IDs by allowing dashboard schema tiles to provide unique ids to the grid layout

@filipgutica filipgutica requested a review from a team as a code owner February 11, 2025 05:03
@filipgutica filipgutica changed the title fix: handle added event fix: handle adding widget Feb 11, 2025
@filipgutica filipgutica force-pushed the fix/handle-added-widgets branch from c268a88 to 223b37c Compare February 11, 2025 07:18
@filipgutica filipgutica force-pushed the fix/handle-added-widgets branch from 223b37c to 1674048 Compare February 11, 2025 07:22
@filipgutica filipgutica requested review from kongponents-bot and a team as code owners February 11, 2025 07:22
Copy link
Contributor

@adorack adorack left a comment

Choose a reason for hiding this comment

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

I'd like to review.

adorack
adorack previously approved these changes Feb 11, 2025
<DashboardRenderer
ref="dashboardRendererRef"
can-remove-tiles
Copy link
Contributor

Choose a reason for hiding this comment

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

Mostly a note for later: I think we should combine this and draggable into one editable prop -- or possibly even just check if the event listeners are defined rather than having separate props.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just combined them to a can-edit prop

@filipgutica filipgutica changed the title fix: handle adding widget fix: handle adding/removing tiles (widgets) Feb 12, 2025
@filipgutica filipgutica force-pushed the fix/handle-added-widgets branch 3 times, most recently from 77a09cf to a02e33e Compare February 12, 2025 06:54
@filipgutica filipgutica force-pushed the fix/handle-added-widgets branch from a02e33e to b88f1d7 Compare February 12, 2025 07:08
@filipgutica filipgutica force-pushed the fix/handle-added-widgets branch from 9d97ff6 to fdffa2a Compare February 12, 2025 15:15
@filipgutica filipgutica force-pushed the fix/handle-added-widgets branch from 09cf80f to 8ebfceb Compare February 12, 2025 20:55
@filipgutica filipgutica force-pushed the fix/handle-added-widgets branch from c9e4779 to 9d8f145 Compare February 13, 2025 01:13
@filipgutica filipgutica force-pushed the fix/handle-added-widgets branch from a51454d to b916631 Compare February 13, 2025 19:06
@filipgutica filipgutica force-pushed the fix/handle-added-widgets branch from b916631 to 869eaba Compare February 13, 2025 19:19
Copy link
Contributor

@adorack adorack left a comment

Choose a reason for hiding this comment

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

A handful of minor comments -- looking at the other preview.

@@ -603,6 +603,7 @@ describe('<DashboardRenderer />', () => {
tileHeight: 167,
tiles: [
{
id: 0,
Copy link
Contributor

Choose a reason for hiding this comment

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

Ahhh, this is to make sure the test IDs stay consistent?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, the ids became random uuids since they aren't provided, manually set them to 0, 1, 2... in order to keep the test sane.

@filipgutica filipgutica merged commit 82359a2 into main Feb 13, 2025
12 checks passed
@filipgutica filipgutica deleted the fix/handle-added-widgets branch February 13, 2025 21:57
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.

2 participants