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

Dashboard tiles layout #1520

Merged
merged 23 commits into from
May 21, 2024
Merged

Dashboard tiles layout #1520

merged 23 commits into from
May 21, 2024

Conversation

ptbrowne
Copy link
Collaborator

Tiles layout

In this PR is implemented the tiles layout where the user can move and resize the charts at will.
It makes use of react-grid-layout under the hood and the react grid layouts are saved under
the chart config.

To be able to have charts inside blocks that are resizable, I had to change how the aspect ratio
was dealt with within the charts: previously the charts would all have an aspect ratio (deep
within them), and would compute their height based on this. This would make it impossible to
have charts inside fixed boxes, since the height of the chart could overflow the box because
of the aspect ratio.

This is why I had to remove the constrain of aspect ratio for the dashboard tiles layout. This
was done through the removal of the aspect ratio in all charts. Now they all render based
on the available space they have.

To keep the previous behavior, the chart container now puts a default "5 / 2" aspect ratio,
which ensures that charts have the same space as before. It would be possible to have
a different aspect ratio for each chart via CSS.

For the dashboard layout, the chart container removes the aspect ratio constraint, and the
chart containers take all the space available.

@ptbrowne ptbrowne requested a review from bprusinowski as a code owner May 21, 2024 15:25
Copy link

vercel bot commented May 21, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
visualization-tool ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 21, 2024 8:13pm

Copy link
Collaborator

@bprusinowski bprusinowski left a comment

Choose a reason for hiding this comment

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

Fantastic work, I am amazed how well this works ✨ 👏

While playing around with the deployment preview, I've noticed two things:

  • it's almost impossible to interact with the chart in the Tiles Dashboard (e.g. switch to table chart or opening the metadata panel). It seems to work when I click and hold, drag the chart a bit, and then release 😅 I wonder if drag handles like in other layouts wouldn't make this more coherent?
tiles.layout.1080p.mov
  • I managed to make an invisible scatterplot (first chart) – I guess it's not a big issue, as creators will see what they publish beforehand, but still this is not possible for line or column chart 👀
Screenshot 2024-05-21 at 17 57 19

app/charts/shared/chart-dimensions.tsx Outdated Show resolved Hide resolved
app/components/chart-with-filters.tsx Outdated Show resolved Hide resolved
app/charts/shared/use-width.tsx Outdated Show resolved Hide resolved
app/utils/use-resize-observer.ts Show resolved Hide resolved
@ptbrowne
Copy link
Collaborator Author

ptbrowne commented May 21, 2024

Fantastic work, I am amazed how well this works ✨ 👏

Thanks 😊

I wonder if drag handles like in other layouts wouldn't make this more coherent?

Yes I will look into that, thanks for the feedback.

I managed to make an invisible scatterplot (first chart)

Yes, not sure if it should be solved. I think people would be aware yes :)

@ptbrowne
Copy link
Collaborator Author

I could add the drag handle and react-grid-layout has a option so that only elements matching a particular class name are used for drag 👍

@ptbrowne
Copy link
Collaborator Author

Checked the red tests and they work locally (apart from an old config which uses a datasets for which there are duplicated cubes on int). Sometimes we need to find out why we have those flaky tests :/

@ptbrowne ptbrowne merged commit e591cd0 into main May 21, 2024
3 of 4 checks passed
@ptbrowne ptbrowne deleted the feat/dashboard-grid-layout branch May 21, 2024 20:06
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