Re-implements the Bubble widget. #7882
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The old Bubble widget design was complicated and used a GridLayout with multiple dummy widgets solely to get an arrow placed as desired. Multiple magic numbers like 99 columns or 99 rows were undocumented and unintuitive. Probably some of these numbers even add unnecessary limitations to the Bubble widget. The Bubble widget was fused with the 'BubbleContent' which by design makes the Bubble widget feel very unflexible. Sizing the Bubble according to the content was unintuitive if not even impossible for some cases and the arrow positioning was relying on magic numbers as well. Lastly, the old implementation lacked test coverage with very few tests that did not even cover the core logic of the widget. The new implementation reduces the core logic (formerly "on_arrow_pos", ~100 lines of code) to ~30 lines plus a well documented lookup structure without magic but intuitive values. Furthermore, it adds proper inline documentation for maintainers. Furthermore, completely flexible arrow positioning was implemented and allows free positioning of the arrow to all sides of the content by selecting a (relative) position on the Bubble widget border. The BubbleContent widget is a seperate styled BoxLayout and the code and documentation makes clear that any other Widet or Layout can be placed as content. The background_color, border and border_auto_scale properties have been attached to the BubbleContent instead of the Bubble itself which removes the unnecessary connections between the Bubble itself and it's BubbleContent. Properties, that refer to multiple dimensions are cleanly integrated through ReferenceListProperties. The rework of the Bubble widget fixes the issue kivy#4348 which had been opened in 2016 and remained unsolved. Minimal changes are necessary for existing applications using the Bubble widget: - A BubbleContent (can be any Layout or a single Widget) is added as only child to the Bubble (similarly to the ActionBar/ActionView widgets). - The background_color, border and border_auto_scale properties have been removed from the Bubble as they belong to the BubbleContent Signed-off-by: AnthonyZimmermann <anthony.zimmermann@protonmail.com>
Core functionality tests: - Rendering - Add/remove content - Error handling if two child widgets are added to the Bubble - Test BubbleContent, BubbleButton Arrow positioning and Bubble sizing: - parameterized tests for predefined arrow_pos arrow positioning (12 tests) - parameterized tests for flexible flex_arrow_pos arrow positioing (40 tests) Signed-off-by: AnthonyZimmermann <anthony.zimmermann@protonmail.com>
The textinput uses a Bubble for "cut", "copy", "paste" functionality. Three example scripts and kv files contained the Bubble widget. Signed-off-by: AnthonyZimmermann <anthony.zimmermann@protonmail.com>
|
Thanks for opening your first pull request here! 💖 Please check out our contributing guidelines. |
AnthonyZi
commented
May 17, 2022
|
|
||
| def adjust_position(self): | ||
| if self.limit_to is not None and not self._temporarily_ignore_limits: | ||
| if self.limit_to is EventLoop.window: |
There was a problem hiding this comment.
This part was mainly copied from the old implementation. I use the Bubble widget mainly in a Scatter Plane where this logic does somehow not really make sense. Maybe someone else can verify that this code still "usefull" in other use cases.
AnthonyZi
commented
May 17, 2022
| add = self_arrow_layout.add_widget | ||
| for widg in arrow_list: | ||
| add(widg) | ||
| if not (lim_x > self.x and lim_right < self.right): |
There was a problem hiding this comment.
Adapted this part a little to make it only "intelligent" if there is a possible better solution (widget is completely within the limits).
akshayaurora
added
Component: core-widget
|
Hey, this is my first PR here. I am wondering if there is something blocking the review process. Is someone/something waiting for me to take some action? Tanks in advance! |
Updates documentation: - Adds further documentation - More details about 'versionadded' and 'versionchanged' Little improvement/fix (found during update of the documentation): - Update the arrow image size information on arrow_image change.
akshayaurora
approved these changes
May 30, 2022
|
Congrats on merging your first pull request! 🎉🎉🎉 |
|
@AnthonyZi thanks 👍 |
7 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit comprises three commits:
The old Bubble widget design was complicated and used a GridLayout with
multiple dummy widgets solely to get an arrow placed as desired.
Multiple magic numbers like 99 columns or 99 rows were undocumented
and unintuitive. Probably some of these numbers even add unnecessary
limitations to the Bubble widget.
The Bubble widget was fused with the 'BubbleContent' which by design
makes the Bubble widget feel very unflexible.
Sizing the Bubble according to the content was unintuitive if not even
unpossible for some cases and the arrow positioning was relying on magic
numbers as well. Lastly, the old implementation lacked test coverage
with very few tests that did not even cover the core logic of the
widget.
The new implementation reduces the core logic (formerly "on_arrow_pos",
~100 lines of code) to ~30 lines plus a well documented lookup structure
without magic but intuitive values. Furthermore, it adds proper inline
documentation for maintainers.
This PR also adds unit tests for the core logic of the Bubble widget.
Maintainer merge checklist
Component: xxxlabel.api-deprecationorapi-breaklabel.release-highlightlabel to be highlighted in release notes.versionadded,versionchangedas needed.