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

Add scroll area element #1072

Merged

Conversation

eli-kha
Copy link

@eli-kha eli-kha commented Jun 24, 2023

Scroll Area is a Quasar component that offers a neat way of customizing the scrollbars by encapsulating your content.

It is particularly useful when you want to allow large content in a smaller element.
Some added benefits are better controls over the scroll interface and programmatic access to the scroll movement.

This PR introduces a new element with matching CSS definitions based on the feedback I got here.
In addition, I want to expose the scroll event and the setScrollPosition and setScrollPercentage

EDIT:The event and functions are not working ATM.

@eli-kha eli-kha closed this Jun 24, 2023
@falkoschindler
Copy link
Contributor

@eli-kha Thanks for this PR!
But why did you close it? Looks like you changed your mind and don't want to have this reviewed and merged.

@eli-kha eli-kha reopened this Jun 29, 2023
@eli-kha
Copy link
Author

eli-kha commented Jun 29, 2023

@falkoschindler I wasn't sure if it was better to seek guidance in a PR or the discussion.
I updated the comment and reopened the PR.

The code is currently broken and I couldn't get the scroll event to trigger a callback in my testing code.

Some guidance on how to get it to work or what to test would be much appreciated

@eli-kha eli-kha changed the title Add scroll area element - for consult Add scroll area element Jul 6, 2023
@eli-kha
Copy link
Author

eli-kha commented Jul 6, 2023

I resolved the event issues and feel ready for a proper review

@eli-kha eli-kha marked this pull request as ready for review July 6, 2023 18:52
# Conflicts:
#	website/documentation.py
@falkoschindler falkoschindler self-requested a review July 7, 2023 09:07
@falkoschindler falkoschindler changed the base branch from main to scroll-area July 7, 2023 09:25
@falkoschindler falkoschindler merged commit 8fe2739 into zauberzeug:scroll-area Jul 7, 2023
1 check passed
@falkoschindler falkoschindler added this to the 1.3.0 milestone Jul 7, 2023
@falkoschindler falkoschindler added the enhancement New feature or request label Jul 7, 2023
Copy link
Contributor

@falkoschindler falkoschindler left a comment

Choose a reason for hiding this comment

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

@eli-kha Thanks again for this pull request!

I updated the code here and there (see 2d62887) and merged it. 🙂

  • Now the ScrollEventArguments follow the Python naming convention instead of the original Quasar attribute names. This makes the scroll area implementation a bit longer, but feels more natural for the user.
  • The _handle_scroll method should use handle_event from the "events" module. This allows using async handlers and automatically uses the right UI context, e.g. when adding new UI elements.
  • I renamed the set_scroll_position to the simpler scroll_to. Furthermore I introduced separate parameters pixels and percent to distinguish both units. Using the type alone seems a bit unreliable.
  • The default size of 200x200px seems a bit arbitrary. But I understand that giving no initial size causes the scroll area to collapse to zero size, which might confuse the user. So I decided to use the same somewhat arbitrary values of 100% width and 16rem height like for ui.aggrid.

@eli-kha eli-kha deleted the add-scroll-area-element-1 branch August 12, 2023 20:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants