Fix#4672 : Select_all,Copy, Paste options reachable on android when textinput is upper/top widget #4761
Conversation
| @@ -80,6 +80,9 @@ class BubbleButton(Button): | |||
| Rather use this BubbleButton widget that is already defined and provides a | |||
| suitable background for you. | |||
| ''' | |||
| def __init__(self, **kwargs): | |||
| super(BubbleButton, self).__init__(**kwargs) | |||
| self.always_release = BooleanProperty(False) | |||
There was a problem hiding this comment.
properties are to be used at class level.
There was a problem hiding this comment.
@akshayaurora sorry for the delay . I realized that the always_release should be a class property and hence changed bubble.py
dessant
added
the
awaiting-reply
saqib1707
force-pushed
the
bubble-textinput-problem
branch
from
f59ad25
to
3207bb1
Compare
| @@ -1402,7 +1402,8 @@ def on_touch_up(self, touch): | |||
| # show Bubble | |||
| win = EventLoop.window | |||
| if self._selection_to != self._selection_from: | |||
| self._show_cut_copy_paste(touch.pos, win) | |||
| touch_rel_pos = (touch.pos[0] - self.x, touch.pos[1] - self.y) | |||
There was a problem hiding this comment.
There should be no adjustment made outside the cut copy paste function as positioning is done in it.
Doing it in multiple places will lead to confusion.
There was a problem hiding this comment.
But the show_cut_copy_paste function accepts relative positions because in one place local (wrt the widget) position was provided to the cut_copy_paste function while here it was absolute(wrt the window) .
There was a problem hiding this comment.
The show_cut_copy_paste function is supposed to manage the display of the bubble,
thus it should be taking care of the conversion if any, this has the added advantage that we won't have the conversion in many places.
| @@ -1593,27 +1594,27 @@ def _show_cut_copy_paste(self, pos, win, parent_changed=False, | |||
| win_size = win.size | |||
| bubble_pos = (t_pos[0], t_pos[1] + inch(.25)) | |||
|
|
|||
| if (bubble_pos[0] - bubble_hw) < 0: | |||
| if (bubble_pos[0] - bubble_hw + self.x) < 0: | |||
There was a problem hiding this comment.
Adding the x/y of the widget to the check for bubble is not the correct solution here. The widget could be off screen or half visible...
There was a problem hiding this comment.
@akshayaurora got your point about half-visiblity but when it is off-screen (not on the screen) then no touch is made on the textinput so anyway no bubble is going to appear.
| @@ -80,7 +80,7 @@ class BubbleButton(Button): | |||
| Rather use this BubbleButton widget that is already defined and provides a | |||
| suitable background for you. | |||
| ''' | |||
| pass | |||
| always_release = BooleanProperty(True) | |||
There was a problem hiding this comment.
Please see #4672 (comment).
There was a problem hiding this comment.
@dessant if the always_release is not explicitly made True for BubbleButton class (as highlighted above) then the on_release event won't be triggered by the button here https://github.com/kivy/kivy/blob/master/kivy/data/style.kv#L202-L217 making the bubbles non-functioning. Other way would be to use on_press instead of on_release in style.kv...
There was a problem hiding this comment.
on_press would just sidestep the bug, the same way always_release = True would. We're looking for a solution which lets users set both True and False for always_release.
dessant
removed
the
awaiting-reply
…extinput is upper/top widget
…l modifications in textinput.py
saqib1707
force-pushed
the
bubble-textinput-problem
branch
from
3207bb1
to
9b436bb
Compare
| @@ -454,7 +454,7 @@ class TextInput(FocusBehavior, Widget): | |||
| 'on_quad_touch') | |||
|
|
|||
| def __init__(self, **kwargs): | |||
| self.always_release_bubble = kwargs.get('always_release_bubble', False) | |||
| self.always_release_bubble = kwargs.get('always_release_bubble', True) | |||
There was a problem hiding this comment.
I think this is just hiding the behavior change, but we are still able to see it. 😁 Do not modify always_release in any form.
There was a problem hiding this comment.
But u said that there should be a way so that users can set always_release for the BubbleButton object. From here https://github.com/kivy/kivy/blob/master/kivy/uix/behaviors/button.py#L168-L169 if self.dispatch('on_release') event has to be triggered then either always_release should be True or self.collide_point(*touch.pos) should be True.
I created a new variable which can be set by the users .Other way would be to change the touch.pos in local or windows co-ordinates.(not sure about the second option)
There was a problem hiding this comment.
I think the keyword argument is not strictly needed to fix the bug you were trying to fix. That's a different feature and customization option. What i was saying is, you should not care about the value of always_release, and most importantly don't change the default value yourself. I'm going to close this pr because it's a collective waste of time for everyone involved, please open a new pr if you have a fix that doesn't merely hide the bug.
The pos argument of _show_cut_copy_paste function in textinput.py file has to be relative , so changed that to relative(to the widget) by subtracting self.y (position of textinput widget w.r.t the window). Also in function on_touch_up in textinput.py the call to _show_cut_copy_paste has been made using absolute position, so introduced a variable touch_rel_pos to pass the relative touch position to _show_cut_copy_paste.
To make the textinput bubble respond on touching , added a always_release=True in the TextInputCutCopyPaste Class.