-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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 colored emoji support to TextInput
#8491
base: master
Are you sure you want to change the base?
Conversation
3fc5250
to
6820e39
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was tempted to press "merge", but I'm also concerned about breaking an undocumented behavior that someone might have used for more than a decade.
If someone (and I have, but I can live with that), created a custom TextInput
, with custom KV
code, without using the default implem, now that developer may have something like that in their codebase:
<MyCustomTextInput>:
canvas.before:
Color:
rgba: self.background_color
Rectangle:
pos: self.pos
size: self.size
Color:
rgba:
(self.cursor_color
if self.focus and not self._cursor_blink
and int(self.x + self.padding[0]) <= self._cursor_visual_pos[0] <= int(self.x + self.width - self.padding[2])
else (0, 0, 0, 0))
Rectangle:
pos: self._cursor_visual_pos
size: root.cursor_width, -self._cursor_visual_height
Color:
rgba: 1, 0, 0, 1
This leverages the thing that Kivy's TextInput label has been rendered as full white, and the color has been applied via canvas for a very long time.
If we apply this PR as-is, in a feature release, someone will certainly not so happy when discovers that all the TextInput
labels are rendered in a color different from the expected one (red in this case).
... so, here we have 2 proposals:
- Add a
experimental_color_in_corelabel
(or one with a better name) property that defaults toFalse
(so the label color is applied viacanvas
by default), and merge this feature for2.3.0
- Wait after the
2.3.0
release, merge the PR as-is and break the "undocumented API", as we already likely need to increment our major version (3.x.x
)
... since we're really near to 2.3.0.rc1
, I would suggest avoiding the complexity of having an experimental
switch.
If someone needs this feature out there (but is useless without a proper logic for font fallback), can likely live by using the master
version of Kivy for the next year or so.
So do you suggest moving this feature to |
@misl6 If the intention is to maintain support for the existing (undocumented) behavior and only that, an approach similar to this could be used. What do you think? last_instruction = self.canvas.before.children[-1]
if isinstance(last_instruction, Color) and tuple(last_instruction.rgba) == (1, 1, 1, 1):
kw["color"] = (
self.disabled_foreground_color if self.disabled
else (self.hint_text_color if hint else self.foreground_color)
) |
This is pretty hacky, even if may cover our need. IMHO: Better to wait for the chance to break the API (and the undocumented behaviors). |
Ok, agreed! 👍 |
This is a PR based on and replacement of #7997 (mentioned as an orphan PR by @Julian-O).
@RobinPicard Thanks for the original PR!
Test code:
On Windows, using
seguiemj
:Maintainer merge checklist
Component: xxx
label.api-deprecation
orapi-break
label.release-highlight
label to be highlighted in release notes.versionadded
,versionchanged
as needed.