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

Small clean up and solves #150 #151

Merged
merged 10 commits into from
Nov 2, 2021
Merged

Small clean up and solves #150 #151

merged 10 commits into from
Nov 2, 2021

Conversation

hbcarlos
Copy link
Member

@hbcarlos hbcarlos commented Oct 29, 2021

This PR solves multiple issues.

Commits:

1: Removes versions from the binder env file to install latest.

2: Solve #143
* Remaining comments from #142.

3: Solves #150:
* Adds the scrolled distance to the y offset.

4 and 6: Improves the size of a new cell
* When adding a new cell to the grid, the size is too small.
* Gets the size from the original cell and adds 40px to the height (the size of the toolbar).

5 and 8: Makes the grid fit the size of the panel
* Once the grid is rendered, gets the size of the panel and modifies the min-height of the grid to fit the panel.
* I think this improves the UX by having always the grid visible, and removing the resize behavior when adding the first cell to the grid.
* This doesn't affect the final look of the layout when rendering with voila.
* Problem: we don't have the helper anymore (Maybe I can add it on top).

9: Improves the scroll behavior
* Remove the horizontal scroll since is not necessary.
* Allows scrolling when adding a new cell.

Screencast:

Peek.2021-11-01.10-20.mp4

@hbcarlos hbcarlos marked this pull request as draft October 29, 2021 13:53
@hbcarlos hbcarlos marked this pull request as ready for review November 1, 2021 08:30
@hbcarlos hbcarlos marked this pull request as draft November 1, 2021 09:21
@hbcarlos hbcarlos marked this pull request as ready for review November 1, 2021 15:58
@jtpio jtpio added the bug Something isn't working label Nov 2, 2021
@@ -10,5 +10,5 @@ dependencies:
- pandas
- scipy
- voila
- voila-gridstack=0.2.0
- jupyterlab=3
- voila-gridstack
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we could remove voila-gridstack from the environment.yml? And add pip install . in a postBuild file.

That way we could more easily test PRs on Binder.

@jtpio
Copy link
Member

jtpio commented Nov 2, 2021

@hbcarlos
Copy link
Member Author

hbcarlos commented Nov 2, 2021

Thanks, Jeremy!

@jtpio
Copy link
Member

jtpio commented Nov 2, 2021

Just checking: it sounded like the issue mentioned in #122 (comment) is not related to this PR?

Then we can track it separately to not block the merge.

@hbcarlos
Copy link
Member Author

hbcarlos commented Nov 2, 2021

No, is not related to this PR. Also is not critical, and I don't think users will see this issue since it happens when stopping and running JupyterLab several times.

Copy link
Member

@jtpio jtpio left a comment

Choose a reason for hiding this comment

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

Thanks!!

@jtpio jtpio merged commit 092c7be into voila-dashboards:master Nov 2, 2021
@jtpio
Copy link
Member

jtpio commented Nov 2, 2021

I'll resume #148 so we can release this change and the other ones that were merged recently.

@hbcarlos hbcarlos deleted the vg-scroll branch November 2, 2021 15:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Can't drag cell directly to lower parts of scrolled down grid. Clean up following #142
2 participants