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

Gbm001 refactor buttons #1139

Merged
merged 16 commits into from
Dec 13, 2021
Merged

Conversation

gbm001
Copy link
Contributor

@gbm001 gbm001 commented Dec 8, 2021

Description of the problem

Refactoring of the button.py and button.kv files. The original desire was just to fix the display of disabled buttons, but it escalated...

#1097

Disabled buttons should not have backgrounds if the original button did not have a background. The colour of the background should be consistent with the background colour of the buttons.

Reproducing the problem

The Kitchen Sink demo shows the mess that disabled buttons are in the current version, and screenshots can be found in #1097 . Otherwise, everything looks the same?

Screenshots of the problem

image

Description of Changes

This is essentially a substantial refactoring of buttons. The number of classes has been massively reduced; 90% of behaviour is now in a single BaseButton class which includes the behaviour of the old BasePressedButton class (since all buttons included this anyway). Three mix-in classes add either a text label, an icon, or both in a BoxLayout. The class ButtonElevationBehaviour adds elevation behaviour for the two buttons that use it (in combination with a specific elevation class, which unfortunately introduces a diamond into their inheritance as both inherit from CommonElevationBehaviour).

Using mix-ins for the content is not ideal (fails to obey Composition over Inheritance). A simple approach would just be to always have both an icon and a text label, and hide when unused; alternatively, you could always have the BoxLayout container and add the appropriate contents (which would be more flexible).

Most colours on the buttons are now actually set by underscore-prefixed ColorProperty attributes; for example, md_bg_color can be None, while the actual button color is bound to _md_bg_color. There are _default prefixed names used for button class creation e.g. _default_md_bg_color. If a user sets md_bg_color, then this is always used making over-riding button colour simple and easy both for the user and in the code. If not set, then _md_bg_color is set to _default_md_bg_color which could be None (to use the primary color), [0, 0, 0, 0] (for a button with a transparent background) or any other colour. There are equivalents for md_bg_color_disabled, line_color and line_color_disabled for the disabled fill, the border and the disabled border, respectively.

A similar system is used for the text and icon colours; by default, if icon_color is not set then _icon_color is set to _text_color.

New colours are added the theming.py which are the disabled versions of the primary colour and the opposite primary colour.

Opposite colours are not yet supported.

Screenshots of the solution to the problem

image

@gbm001
Copy link
Contributor Author

gbm001 commented Dec 8, 2021

I've just seen in the ToDo list that there is a desire to combine all MDButton classes into a single MDButton class with a 'type' attribute. This would certainly be doable. It might be a bit tricky with the elevation buttons (MDRaisedButton and MDFloatingActionButton), which use different elevation classes, but doable. Also 'type' is already used for the MDFloatingActionButton for the M3 button size but this could be a different option.

@HeaTTheatR
Copy link
Member

@gbm001 Buttons are a huge package. Therefore, in order to accept any changes, tests must be developed for all classes of buttons.

@HeaTTheatR
Copy link
Member

@gbm001 In any case, I cannot accept these changes without detailed documentation for each new class and tests.

@gbm001
Copy link
Contributor Author

gbm001 commented Dec 8, 2021

@gbm001 In any case, I cannot accept these changes without detailed documentation for each new class and tests.

More than happy to do tests - if nothing else, it ties down the specification of what the classes are supposed to do. Since it looks like there are currently almost no tests in KivyMD, and as far as I know there are no tests of user-facing components, what sort of tests would you like? Just a test application with a reasonable range of classes (like the kitchen sink demo but with more examples of customizations and options)? Could be combined with a 'correct' reference image (from a screenshot) for each case. Unfortunately, such testing would be manual rather than automatic, unless you have a plan for automated testing?

There are no new user-facing classes - only internal classes (and far fewer of them), and while there are probably a few minor documentation changes that I've missed (I haven't gone through carefully yet as I'm assuming there will be further changes) I have added documentation for any new non-private properties (of which there are very few).

@HeaTTheatR
Copy link
Member

@gbm001

from kivy.lang import Builder
from kivy.utils import get_hex_from_color

from kivymd.app import MDApp
from kivymd.color_definitions import colors
from kivymd.uix.button import (
    MDRoundFlatButton,
    MDIconButton,
    MDFloatingActionButton,
    MDFlatButton,
    MDRaisedButton,
    MDRectangleFlatButton,
    MDRectangleFlatIconButton,
    MDRoundFlatIconButton,
    MDFillRoundFlatButton,
    MDFillRoundFlatIconButton,
    MDTextButton,
)

KV = """
MDScreen:

    MDBoxLayout:
        orientation: "vertical"
        padding: 20
        spacing: "10dp"

        ScrollView:
    
            MDBoxLayout:
                orientation: "vertical"
                padding: 20
                spacing: "10dp"
                adaptive_height: True
    
                # MDIconButton
    
                MDLabel:
                    text: "MDIconButton from KV"
                    adaptive_height: True
                    halign: "center"
                    font_style: "H6"
    
                MDSeparator:
        
                MDBoxLayout:
                    padding: 20
                    spacing: "10dp"
                    adaptive_height: True
        
                    MDIconButton:
                        id: btn_1
                        icon: "android"
                        user_font_size: "24sp"
        
                    MDIconButton:
                        id: btn_2
                        icon: "android"
                        user_font_size: "36sp"
        
                    MDIconButton:
                        id: btn_3
                        icon: "android"
                        user_font_size: "48sp"
        
                    MDIconButton:
                        icon: "android"
                        user_font_size: "56sp"
                        disabled: True

                MDBoxLayout:
                    adaptive_size: True

                    MDFlatButton:
                        text: "SET ICON COLOR"
                        on_release:
                            btn_1.theme_text_color = "Custom"
                            btn_1.text_color = (0, 0, 1, 1)
                            btn_2.theme_text_color = "Custom"
                            btn_2.text_color = (0, 0, 1, 1)
                            btn_3.theme_text_color = "Custom"
                            btn_3.text_color = (0, 0, 1, 1)
        
                    MDFlatButton:
                        text: "DISABLED"
                        on_release:
                            btn_1.disabled = True
                            btn_2.disabled = True
                            btn_3.disabled = True
        
                    MDFlatButton:
                        text: "UNDISABLED"
                        on_release:
                            btn_1.disabled = False
                            btn_2.disabled = False
                            btn_3.disabled = False

                    MDFlatButton:
                        text: "SET BG COLOR"
                        on_release:
                            btn_1.md_bg_color = (0, 1, 0, 1)
                            btn_2.md_bg_color = (1, 1, 0, 1)
                            btn_3.md_bg_color = (1, 0, 0, 1)

                    MDFlatButton:
                        text: "SET STYLE"
                        on_release:
                            app.theme_cls.theme_style = "Dark" \
                            if app.theme_cls.theme_style == "Light" \
                            else "Light"

                    MDFlatButton:
                        text: "SET PALETTE"
                        on_release:
                            app.theme_cls.primary_palette = "Green"

                MDBoxLayout:
                    orientation: "vertical"
                    padding: 20
                    spacing: "10dp"
                    adaptive_height: True
                    md_bg_color: 0, 0, 0, .2

                    MDLabel:
                        text: "MDIconButton from Python"
                        adaptive_height: True
                        halign: "center"
                        font_style: "H6"
            
                    MDBoxLayout:
                        id: python_box
                        padding: 20
                        spacing: "10dp"
                        adaptive_height: True

                MDSeparator:

                # MDFloatingActionButton

                MDLabel:
                    text: "MDFloatingActionButton from KV"
                    adaptive_height: True
                    halign: "center"
                    font_style: "H6"
            
                MDBoxLayout:
                    padding: 20
                    spacing: "10dp"
                    adaptive_height: True
        
                    MDFloatingActionButton:
                        id: btn_4
                        icon: "git"
        
                    MDFloatingActionButton:
                        id: btn_5
                        icon: "android"
        
                    MDFloatingActionButton:
                        id: btn_6
                        icon: "android"
        
                    MDFloatingActionButton:
                        icon: "android"
                        disabled: True
        
                MDBoxLayout:
                    adaptive_size: True

                    MDFlatButton:
                        text: "SET ICON COLOR"
                        on_release:
                            btn_4.theme_text_color = "Custom"
                            btn_4.text_color = (1, 0, 1, 1)
                            btn_5.theme_text_color = "Custom"
                            btn_5.text_color = (0, 1, 1, 1)
                            btn_6.theme_text_color = "Custom"
                            btn_6.text_color = (1, 0, 0, 1)
        
                    MDFlatButton:
                        text: "DISABLED"
                        on_release:
                            btn_4.disabled = True
                            btn_5.disabled = True
                            btn_6.disabled = True
        
                    MDFlatButton:
                        text: "UNDISABLED"
                        on_release:
                            btn_4.disabled = False
                            btn_5.disabled = False
                            btn_6.disabled = False

                    MDFlatButton:
                        text: "SET BG COLOR"
                        on_release:
                            btn_4.md_bg_color = (0, 1, 0, 1)
                            btn_5.md_bg_color = (1, 1, 0, 1)
                            btn_6.md_bg_color = (1, 0, 0, 1)

                    MDFlatButton:
                        text: "SET STYLE"
                        on_release:
                            app.theme_cls.theme_style = "Dark" \
                            if app.theme_cls.theme_style == "Light" \
                            else "Light"

                    MDFlatButton:
                        text: "SET PALETTE"
                        on_release: app.theme_cls.primary_palette = "Yellow"

                MDBoxLayout:
                    orientation: "vertical"
                    padding: 20
                    spacing: "10dp"
                    adaptive_height: True
                    md_bg_color: 0, 0, 0, .2
        
                    MDLabel:
                        text: "MDFloatingActionButton from Python"
                        adaptive_height: True
                        halign: "center"
                        font_style: "H6"
            
                    MDBoxLayout:
                        id: python_box_2
                        padding: 20
                        spacing: "10dp"
                        adaptive_height: True

                # MDFlatButton

                MDSeparator:

                MDLabel:
                    text: "MDFlatButton from KV"
                    adaptive_height: True
                    halign: "center"
                    font_style: "H6"
            
                MDBoxLayout:
                    padding: 20
                    spacing: "10dp"
                    adaptive_height: True

                    MDFlatButton:
                        id: btn_7
                        text: "MDFlatButton"

                    MDFlatButton:
                        id: btn_8
                        text: "MDFlatButton"
                        font_style: "H5"
        
                    MDFlatButton:
                        text: "MDFlatButton"
                        disabled: True
        
                MDBoxLayout:
                    adaptive_size: True

                    MDFlatButton:
                        text: "SET TEXT COLOR"
                        on_release:
                            btn_7.theme_text_color = "Custom"
                            btn_7.text_color = (0, 0, 1, 1)
                            btn_8.theme_text_color = "Custom"
                            btn_8.text_color = (0, 0, 1, 1)
        
                    MDFlatButton:
                        text: "DISABLED"
                        on_release:
                            btn_7.disabled = True
                            btn_8.disabled = True
        
                    MDFlatButton:
                        text: "UNDISABLED"
                        on_release:
                            btn_7.disabled = False
                            btn_8.disabled = False

                    MDFlatButton:
                        text: "SET STYLE"
                        on_release:
                            app.theme_cls.theme_style = "Dark" \
                            if app.theme_cls.theme_style == "Light" \
                            else "Light"

                MDBoxLayout:
                    orientation: "vertical"
                    padding: 20
                    spacing: "10dp"
                    adaptive_height: True
                    md_bg_color: 0, 0, 0, .2

                    MDLabel:
                        text: "MDFlatButton from Python"
                        adaptive_height: True
                        halign: "center"
                        font_style: "H6"

                    MDBoxLayout:
                        id: python_box_3
                        padding: 20
                        spacing: "10dp"
                        adaptive_height: True

                # MDRaisedButton

                MDSeparator:

                MDLabel:
                    text: "MDRaisedButton from KV"
                    adaptive_height: True
                    halign: "center"
                    font_style: "H6"
            
                MDBoxLayout:
                    padding: 20
                    spacing: "10dp"
                    adaptive_height: True

                    MDRaisedButton:
                        id: btn_9
                        text: "MDRaisedButton"

                    MDRaisedButton:
                        id: btn_10
                        text: "MDRaisedButton"
                        font_style: "H5"
        
                    MDRaisedButton:
                        text: "MDRaisedButton"
                        disabled: True
        
                MDBoxLayout:
                    adaptive_size: True

                    MDFlatButton:
                        text: "SET TEXT COLOR"
                        on_release:
                            btn_9.theme_text_color = "Custom"
                            btn_9.text_color = (1, 0, 1, 1)
                            btn_10.theme_text_color = "Custom"
                            btn_10.text_color = (0, 0, 1, 1)

                    MDFlatButton:
                        text: "SET BG COLOR"
                        on_release:
                            app.theme_cls.primary_palette = "Yellow"
                            #btn_9.md_bg_color = (0, 0, 1, 1)
                            #btn_10.md_bg_color = (1, 0, 1, 1)
        
                    MDFlatButton:
                        text: "DISABLED"
                        on_release:
                            btn_9.disabled = True
                            btn_10.disabled = True
        
                    MDFlatButton:
                        text: "UNDISABLED"
                        on_release:
                            btn_9.disabled = False
                            btn_10.disabled = False

                    MDFlatButton:
                        text: "SET STYLE"
                        on_release:
                            app.theme_cls.theme_style = "Dark" \
                            if app.theme_cls.theme_style == "Light" \
                            else "Light"

                MDBoxLayout:
                    orientation: "vertical"
                    padding: 20
                    spacing: "10dp"
                    adaptive_height: True
                    md_bg_color: 0, 0, 0, .2

                    MDLabel:
                        text: "MDFlatButton from Python"
                        adaptive_height: True
                        halign: "center"
                        font_style: "H6"

                    MDBoxLayout:
                        id: python_box_4
                        padding: 20
                        spacing: "10dp"
                        adaptive_height: True

                # MDRectangleFlatButton

                MDSeparator:

                MDLabel:
                    text: "MDRectangleFlatButton from KV"
                    adaptive_height: True
                    halign: "center"
                    font_style: "H6"
            
                MDBoxLayout:
                    padding: 20
                    spacing: "10dp"
                    adaptive_height: True

                    MDRectangleFlatButton:
                        id: btn_11
                        text: "MDRectangleFlatButton"

                    MDRectangleFlatButton:
                        id: btn_12
                        text: "MDRectangleFlatButton"
                        font_style: "H5"
        
                    MDRectangleFlatButton:
                        text: "MDRectangleFlatButton"
                        disabled: True
        
                MDBoxLayout:
                    adaptive_size: True

                    MDFlatButton:
                        text: "SET TEXT COLOR"
                        on_release:
                            btn_11.theme_text_color = "Custom"
                            btn_11.text_color = (1, 0, 1, 1)
                            btn_12.theme_text_color = "Custom"
                            btn_12.text_color = (0, 0, 1, 1)

                    MDFlatButton:
                        text: "SET LINE COLOR"
                        on_release:
                            btn_11.line_color = (0, 0, 1, 1)
                            btn_12.line_color = (1, 0, 1, 1)
        
                    MDFlatButton:
                        text: "DISABLED"
                        on_release:
                            btn_11.disabled = True
                            btn_12.disabled = True
        
                    MDFlatButton:
                        text: "UNDISABLED"
                        on_release:
                            btn_11.disabled = False
                            btn_12.disabled = False

                    MDFlatButton:
                        text: "SET STYLE"
                        on_release:
                            app.theme_cls.theme_style = "Dark" \
                            if app.theme_cls.theme_style == "Light" \
                            else "Light"

                    MDFlatButton:
                        text: "SET PALETTE"
                        on_release:
                            app.theme_cls.primary_palette = "Yellow"

                MDBoxLayout:
                    orientation: "vertical"
                    padding: 20
                    spacing: "10dp"
                    adaptive_height: True
                    md_bg_color: 0, 0, 0, .2

                    MDLabel:
                        text: "MDRectangleFlatButton from Python"
                        adaptive_height: True
                        halign: "center"
                        font_style: "H6"

                    MDBoxLayout:
                        id: python_box_5
                        padding: 20
                        spacing: "10dp"
                        adaptive_height: True

                # MDRectangleFlatIconButton

                MDSeparator:

                MDLabel:
                    text: "MDRectangleFlatIconButton from KV"
                    adaptive_height: True
                    halign: "center"
                    font_style: "H6"
            
                MDBoxLayout:
                    padding: 20
                    spacing: "10dp"
                    adaptive_height: True

                    MDRectangleFlatIconButton:
                        id: btn_13
                        text: "MDRectangleFlatIconButton"

                    MDRectangleFlatIconButton:
                        id: btn_14
                        text: "MDRectangleFlatIconButton"
                        font_style: "H5"
        
                    MDRectangleFlatIconButton:
                        text: "MDRectangleFlatIconButton"
                        disabled: True
        
                MDBoxLayout:
                    adaptive_size: True

                    MDFlatButton:
                        text: "SET TEXT COLOR"
                        on_release:
                            btn_13.theme_text_color = "Custom"
                            btn_13.text_color = (1, 0, 1, 1)
                            btn_14.theme_text_color = "Custom"
                            btn_14.text_color = (0, 0, 1, 1)

                    MDFlatButton:
                        text: "SET ICON COLOR"
                        on_release:
                            btn_13.icon_color = (0, 0, 1, 1)
                            btn_14.icon_color = (1, 0, 1, 1)

                    MDFlatButton:
                        text: "SET LINE COLOR"
                        on_release:
                            btn_13.line_color = (1, 0, 1, 1)
                            btn_14.line_color = (0, 0, 1, 1)
        
                    MDFlatButton:
                        text: "DISABLED"
                        on_release:
                            btn_13.disabled = True
                            btn_14.disabled = True
        
                    MDFlatButton:
                        text: "UNDISABLED"
                        on_release:
                            btn_13.disabled = False
                            btn_14.disabled = False

                    MDFlatButton:
                        text: "SET STYLE"
                        on_release:
                            app.theme_cls.theme_style = "Dark" \
                            if app.theme_cls.theme_style == "Light" \
                            else "Light"

                    MDFlatButton:
                        text: "SET PALETTE"
                        on_release:
                            app.theme_cls.primary_palette = "Yellow"

                MDBoxLayout:
                    orientation: "vertical"
                    padding: 20
                    spacing: "10dp"
                    adaptive_height: True
                    md_bg_color: 0, 0, 0, .2

                    MDLabel:
                        text: "MDRectangleFlatIconButton from Python"
                        adaptive_height: True
                        halign: "center"
                        font_style: "H6"

                    MDBoxLayout:
                        id: python_box_6
                        padding: 20
                        spacing: "10dp"
                        adaptive_height: True

                # MDRoundFlatButton

                MDSeparator:

                MDLabel:
                    text: "MDRoundFlatButton from KV"
                    adaptive_height: True
                    halign: "center"
                    font_style: "H6"
            
                MDBoxLayout:
                    padding: 20
                    spacing: "10dp"
                    adaptive_height: True

                    MDRoundFlatButton:
                        id: btn_15
                        text: "MDRoundFlatButton"

                    MDRoundFlatButton:
                        id: btn_16
                        text: "MDRoundFlatButton"
                        font_style: "H5"
        
                    MDRoundFlatButton:
                        text: "MDRoundFlatButton"
                        disabled: True
        
                MDBoxLayout:
                    adaptive_size: True

                    MDFlatButton:
                        text: "SET TEXT COLOR"
                        on_release:
                            btn_15.theme_text_color = "Custom"
                            btn_15.text_color = (1, 0, 1, 1)
                            btn_16.theme_text_color = "Custom"
                            btn_16.text_color = (0, 0, 1, 1)

                    MDFlatButton:
                        text: "SET LINE COLOR"
                        on_release:
                            btn_15.line_color = (1, 0, 1, 1)
                            btn_16.line_color = (0, 0, 1, 1)
        
                    MDFlatButton:
                        text: "DISABLED"
                        on_release:
                            btn_15.disabled = True
                            btn_16.disabled = True
        
                    MDFlatButton:
                        text: "UNDISABLED"
                        on_release:
                            btn_15.disabled = False
                            btn_16.disabled = False

                    MDFlatButton:
                        text: "SET STYLE"
                        on_release:
                            app.theme_cls.theme_style = "Dark" \
                            if app.theme_cls.theme_style == "Light" \
                            else "Light"

                    MDFlatButton:
                        text: "SET PALETTE"
                        on_release: app.theme_cls.primary_palette = "Yellow"

                MDBoxLayout:
                    orientation: "vertical"
                    padding: 20
                    spacing: "10dp"
                    adaptive_height: True
                    md_bg_color: 0, 0, 0, .2

                    MDLabel:
                        text: "MDRoundFlatButton from Python"
                        adaptive_height: True
                        halign: "center"
                        font_style: "H6"

                    MDBoxLayout:
                        id: python_box_7
                        padding: 20
                        spacing: "10dp"
                        adaptive_height: True

                # MDRoundFlatIconButton

                MDSeparator:

                MDLabel:
                    text: "MDRoundFlatIconButton from KV"
                    adaptive_height: True
                    halign: "center"
                    font_style: "H6"
            
                MDBoxLayout:
                    padding: 20
                    spacing: "10dp"
                    adaptive_height: True

                    MDRoundFlatIconButton:
                        id: btn_17
                        text: "MDRoundFlatIconButton"

                    MDRoundFlatIconButton:
                        id: btn_18
                        text: "MDRoundFlatIconButton"
                        font_style: "H5"
        
                    MDRoundFlatIconButton:
                        text: "MDRoundFlatIconButton"
                        disabled: True
        
                MDBoxLayout:
                    adaptive_size: True

                    MDFlatButton:
                        text: "SET TEXT COLOR"
                        on_release:
                            btn_17.theme_text_color = "Custom"
                            btn_17.text_color = (1, 0, 1, 1)
                            btn_18.theme_text_color = "Custom"
                            btn_18.text_color = (0, 0, 1, 1)

                    MDFlatButton:
                        text: "SET LINE COLOR"
                        on_release:
                            btn_17.line_color = (1, 0, 1, 1)
                            btn_18.line_color = (1, 0, 0, 1)

                    MDFlatButton:
                        text: "SET ICON COLOR"
                        on_release:
                            btn_17.icon_color = (0, 0, 1, 1)
                            btn_18.icon_color = (1, 0, 1, 1)
        
                    MDFlatButton:
                        text: "DISABLED"
                        on_release:
                            btn_17.disabled = True
                            btn_18.disabled = True
        
                    MDFlatButton:
                        text: "UNDISABLED"
                        on_release:
                            btn_17.disabled = False
                            btn_18.disabled = False

                    MDFlatButton:
                        text: "SET STYLE"
                        on_release:
                            app.theme_cls.theme_style = "Dark" \
                            if app.theme_cls.theme_style == "Light" \
                            else "Light"

                    MDFlatButton:
                        text: "SET PALETTE"
                        on_release: app.theme_cls.primary_palette = "Yellow"

                MDBoxLayout:
                    orientation: "vertical"
                    padding: 20
                    spacing: "10dp"
                    adaptive_height: True
                    md_bg_color: 0, 0, 0, .2

                    MDLabel:
                        text: "MDRoundFlatButton from Python"
                        adaptive_height: True
                        halign: "center"
                        font_style: "H6"

                    MDBoxLayout:
                        id: python_box_8
                        padding: 20
                        spacing: "10dp"
                        adaptive_height: True

                # MDFillRoundFlatButton

                MDSeparator:

                MDLabel:
                    text: "MDFillRoundFlatButton from KV"
                    adaptive_height: True
                    halign: "center"
                    font_style: "H6"
            
                MDBoxLayout:
                    padding: 20
                    spacing: "10dp"
                    adaptive_height: True

                    MDFillRoundFlatButton:
                        id: btn_19
                        text: "MDFillRoundFlatButton"

                    MDFillRoundFlatButton:
                        id: btn_20
                        text: "MDFillRoundFlatButton"
                        font_style: "H5"
        
                    MDFillRoundFlatButton:
                        text: "MDFillRoundFlatButton"
                        disabled: True
        
                MDBoxLayout:
                    adaptive_size: True

                    MDFlatButton:
                        text: "SET TEXT COLOR"
                        on_release:
                            btn_19.theme_text_color = "Custom"
                            btn_19.text_color = (1, 0, 1, 1)
                            btn_20.theme_text_color = "Custom"
                            btn_20.text_color = (0, 0, 1, 1)

                    MDFlatButton:
                        text: "SET PALETTE"
                        on_release:
                            app.theme_cls.primary_palette = "Yellow"

                    MDFlatButton:
                        text: "SET BG COLOR"
                        on_release:
                            btn_19.md_bg_color = (0, 0, 1, 1)
                            btn_20.md_bg_color = (1, 0, 0, 1)
        
                    MDFlatButton:
                        text: "DISABLED"
                        on_release:
                            btn_19.disabled = True
                            btn_20.disabled = True
        
                    MDFlatButton:
                        text: "UNDISABLED"
                        on_release:
                            btn_19.disabled = False
                            btn_20.disabled = False

                    MDFlatButton:
                        text: "SET STYLE"
                        on_release:
                            app.theme_cls.theme_style = "Dark" \
                            if app.theme_cls.theme_style == "Light" \
                            else "Light"

                MDBoxLayout:
                    orientation: "vertical"
                    padding: 20
                    spacing: "10dp"
                    adaptive_height: True
                    md_bg_color: 0, 0, 0, .2

                    MDLabel:
                        text: "MDFillRoundFlatButton from Python"
                        adaptive_height: True
                        halign: "center"
                        font_style: "H6"

                    MDBoxLayout:
                        id: python_box_9
                        padding: 20
                        spacing: "10dp"
                        adaptive_height: True

                # MDFillRoundFlatIconButton

                MDSeparator:

                MDLabel:
                    text: "MDFillRoundFlatIconButton from KV"
                    adaptive_height: True
                    halign: "center"
                    font_style: "H6"
            
                MDBoxLayout:
                    padding: 20
                    spacing: "10dp"
                    adaptive_height: True

                    MDFillRoundFlatIconButton:
                        id: btn_21
                        text: "MDFillRoundFlatIconButton"

                    MDFillRoundFlatIconButton:
                        id: btn_22
                        text: "MDFillRoundFlatIconButton"
                        font_style: "H5"
        
                    MDFillRoundFlatIconButton:
                        text: "MDFillRoundFlatIconButton 333"
                        disabled: True
        
                MDBoxLayout:
                    adaptive_size: True

                    MDFlatButton:
                        text: "SET TEXT COLOR"
                        on_release:
                            btn_21.theme_text_color = "Custom"
                            btn_21.text_color = (1, 0, 1, 1)
                            btn_22.theme_text_color = "Custom"
                            btn_22.text_color = (0, 0, 1, 1)

                    MDFlatButton:
                        text: "SET ICON COLOR"
                        on_release:
                            btn_21.icon_color = (1, 0, 1, 1)
                            btn_22.icon_color = (0, 1, 0, 1)

                    MDFlatButton:
                        text: "SET BG COLOR"
                        on_release:
                            btn_21.md_bg_color = (0, 0, 1, 1)
                            btn_22.md_bg_color = (1, 0, 0, 1)
        
                    MDFlatButton:
                        text: "DISABLED"
                        on_release:
                            btn_21.disabled = True
                            btn_22.disabled = True
        
                    MDFlatButton:
                        text: "UNDISABLED"
                        on_release:
                            btn_21.disabled = False
                            btn_22.disabled = False

                    MDFlatButton:
                        text: "SET STYLE"
                        on_release:
                            app.theme_cls.theme_style = "Dark" \
                            if app.theme_cls.theme_style == "Light" \
                            else "Light"

                    MDFlatButton:
                        text: "SET PALETTE"
                        on_release:
                            app.theme_cls.primary_palette = "Yellow"

                MDBoxLayout:
                    orientation: "vertical"
                    padding: 20
                    spacing: "10dp"
                    adaptive_height: True
                    md_bg_color: 0, 0, 0, .2

                    MDLabel:
                        text: "MDFillRoundFlatIconButton from Python"
                        adaptive_height: True
                        halign: "center"
                        font_style: "H6"

                    MDBoxLayout:
                        id: python_box_10
                        padding: 20
                        spacing: "10dp"
                        adaptive_height: True
"""


class Test(MDApp):
    def build(self):
        return Builder.load_string(KV)

    def on_start(self):
        # MDIconButton
        self.root.ids.python_box.add_widget(
            MDIconButton(
                icon="language-python",
                user_font_size="24sp",
            )
        )
        self.root.ids.python_box.add_widget(
            MDIconButton(
                icon="language-python",
                user_font_size="36sp",
                theme_text_color="Custom",
                text_color=(1, 0, 0, 1),
                md_bg_color=(1, 0, 1, 1),
            )
        )
        self.root.ids.python_box.add_widget(
            MDIconButton(
                icon="language-python",
                user_font_size="48sp",
                theme_text_color="Custom",
                text_color=(0, 0, 1, 1),
            )
        )
        self.root.ids.python_box.add_widget(
            MDIconButton(
                icon="language-python",
                user_font_size="56sp",
                disabled=True,
            )
        )
        # MDFloatingActionButton
        self.root.ids.python_box_2.add_widget(
            MDFloatingActionButton(icon="language-python")
        )
        self.root.ids.python_box_2.add_widget(
            MDFloatingActionButton(
                icon="language-python",
                theme_text_color="Custom",
                text_color=(1, 0, 0, 1),
                md_bg_color=(1, 0, 1, 1),
            )
        )
        self.root.ids.python_box_2.add_widget(
            MDFloatingActionButton(
                icon="language-python",
                theme_text_color="Custom",
                text_color=(0, 0, 1, 1),
            )
        )
        self.root.ids.python_box_2.add_widget(
            MDFloatingActionButton(
                icon="language-python",
                disabled=True,
            )
        )
        # MDFlatButton
        self.root.ids.python_box_3.add_widget(MDFlatButton(text="MDFlatButton"))
        self.root.ids.python_box_3.add_widget(
            MDFlatButton(
                text="MDFlatButton",
                theme_text_color="Custom",
                text_color=(0, 1, 0, 1),
                font_style="H5",
            )
        )
        self.root.ids.python_box_3.add_widget(
            MDFlatButton(
                text="MDFlatButton",
                disabled=True,
            )
        )
        # MDRaisedButton
        self.root.ids.python_box_4.add_widget(
            MDRaisedButton(text="MDRaisedButton")
        )
        self.root.ids.python_box_4.add_widget(
            MDRaisedButton(
                text="MDFlatButton",
                md_bg_color=(0, 0, 1, 1),
                font_style="H5",
            )
        )
        self.root.ids.python_box_4.add_widget(
            MDRaisedButton(
                text="MDFlatButton",
                disabled=True,
            )
        )
        # MDRectangleFlatButton
        self.root.ids.python_box_5.add_widget(
            MDRectangleFlatButton(text="MDRectangleFlatButton")
        )
        self.root.ids.python_box_5.add_widget(
            MDRectangleFlatButton(
                text="MDRectangleFlatButton",
                line_color=(0, 0, 1, 1),
                theme_text_color="Custom",
                text_color=(1, 0, 1, 1),
                font_style="H5",
            )
        )
        self.root.ids.python_box_5.add_widget(
            MDRectangleFlatButton(
                text="MDRectangleFlatButton",
                disabled=True,
            )
        )
        # MDRectangleFlatIconButton
        self.root.ids.python_box_6.add_widget(
            MDRectangleFlatIconButton(
                icon="language-python",
                text="MDRectangleFlatIconButton",
            )
        )
        self.root.ids.python_box_6.add_widget(
            MDRectangleFlatIconButton(
                icon="language-python",
                text="MDRectangleFlatIconButton",
                theme_text_color="Custom",
                text_color=(1, 0, 0, 1),
                font_style="H5",
                line_color=(0, 0, 1, 1),
                icon_color=(1, 1, 0, 1),
            )
        )
        self.root.ids.python_box_6.add_widget(
            MDRectangleFlatIconButton(
                icon="language-python",
                text="MDRectangleFlatIconButton",
                disabled=True,
            )
        )
        # MDRoundFlatButton
        self.root.ids.python_box_7.add_widget(
            MDRoundFlatButton(
                text="MDRoundFlatButton",
            )
        )
        self.root.ids.python_box_7.add_widget(
            MDRoundFlatButton(
                text="MDRoundFlatButton",
                theme_text_color="Custom",
                text_color=(1, 0, 0, 1),
                line_color=(0, 0, 1, 1),
                font_style="H5",
            )
        )
        self.root.ids.python_box_7.add_widget(
            MDRoundFlatButton(
                text="MDRoundFlatButton",
                disabled=True,
            )
        )
        # MDRoundFlatIconButton
        self.root.ids.python_box_8.add_widget(
            MDRoundFlatIconButton(
                icon="language-python",
                text="MDRoundFlatIconButton",
            )
        )
        self.root.ids.python_box_8.add_widget(
            MDRoundFlatIconButton(
                icon="language-python",
                text="MDRoundFlatIconButton",
                theme_text_color="Custom",
                text_color=(1, 0, 0, 1),
                line_color=(1, 0, 1, 1),
                icon_color=(0, 0, 1, 1),
                font_style="H5",
            )
        )
        self.root.ids.python_box_8.add_widget(
            MDRoundFlatIconButton(
                icon="language-python",
                text="MDRoundFlatIconButton",
                disabled=True,
            )
        )
        # MDFillRoundFlatButton
        self.root.ids.python_box_9.add_widget(
            MDFillRoundFlatButton(
                text="MDFillRoundFlatButton",
            )
        )
        self.root.ids.python_box_9.add_widget(
            MDFillRoundFlatButton(
                text="MDFillRoundFlatButton",
                theme_text_color="Custom",
                text_color=(1, 0, 0, 1),
                md_bg_color=(1, 0, 1, 1),
                font_style="H5",
            )
        )
        self.root.ids.python_box_9.add_widget(
            MDFillRoundFlatButton(
                text="MDFillRoundFlatButton",
                disabled=True,
            )
        )
        # MDFillRoundFlatIconButton
        self.root.ids.python_box_10.add_widget(
            MDFillRoundFlatIconButton(
                text="MDFillRoundFlatIconButton",
            )
        )
        self.root.ids.python_box_10.add_widget(
            MDFillRoundFlatIconButton(
                text="MDFillRoundFlatIconButton",
                theme_text_color="Custom",
                text_color=(1, 0, 0, 1),
                md_bg_color=(1, 0, 1, 1),
                icon_color=(0, 0, 1, 1),
                font_style="H5",
            )
        )
        self.root.ids.python_box_10.add_widget(
            MDFillRoundFlatIconButton(
                text="MDFillRoundFlatIconButton",
                disabled=True,
            )
        )


Test().run()

@gbm001
Copy link
Contributor Author

gbm001 commented Dec 8, 2021

Following the addition of this test, I have improved and fixed the implementation of icon colours (which was broken) to match text colours, which has made the contents classes slightly messier (but hopefully I can get rid of those anyway).

Behaviour in the new and old test now varies only in the following ways, which I think are both bugs in the original version:

  • Disabled buttons look different (wrong in the original version, with backgrounds on buttons that don't have backgrounds and erratic fills)
  • Changing the palette wipes out custom background colours/text colours/icon colours in the original version; these are preserved and not overridden in the new version.

kivymd/uix/button/button.py Outdated Show resolved Hide resolved
kivymd/uix/button/button.py Show resolved Hide resolved
Copy link
Member

@HeaTTheatR HeaTTheatR left a comment

Choose a reason for hiding this comment

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

Delete this file, this code is for my personal testing only and has nothing to do with tests in principle.

kivymd/uix/button/button.py Outdated Show resolved Hide resolved
kivymd/uix/button/button.py Show resolved Hide resolved
kivymd/uix/button/button.py Outdated Show resolved Hide resolved
self._icon_color = self.icon_color or self._text_color


class ButtonContentsIconText():
Copy link
Member

Choose a reason for hiding this comment

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

Why is this class not inherited from any widget?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's just a mix-in; it shouldn't be instantiated on it's own but just adds contents to a BaseButton.

I would like to move to a better and more flexible system anyway for adding content to buttons - or possibly move to a simpler system that only supports one or zero icons and one or zero text labels for all buttons.

kivymd/uix/button/button.py Outdated Show resolved Hide resolved
kivymd/uix/button/button.py Outdated Show resolved Hide resolved
kivymd/uix/button/button.py Show resolved Hide resolved
kivymd/uix/button/button.py Outdated Show resolved Hide resolved
kivymd/uix/button/button.py Outdated Show resolved Hide resolved
kivymd/uix/button/button.py Outdated Show resolved Hide resolved
@HeaTTheatR
Copy link
Member

@gbm001
Снимок экрана 2021-12-08 в 22 56 51

@HeaTTheatR
Copy link
Member

@gbm001
111

@gbm001
Copy link
Contributor Author

gbm001 commented Dec 9, 2021

@gbm001 Снимок экрана 2021-12-08 в 22 56 51

This only happens when you first set the buttons to different background colours, and then disable them. The Material Design spec says that you set stuff to 38% opacity. Because the background colours are different, the disabled colours are different (I've used 50% opacity for the dark theme because that was what was in the code before, but I don't know if this is correct).

However, despite being what the spec says, it does look a bit silly. It would be easy enough to just use a single disabled colour (one for light theme, one for dark theme) which could either be 38% of the primary color or just 38% of black (and equivalent for dark theme).

@gbm001
Copy link
Contributor Author

gbm001 commented Dec 9, 2021

@gbm001 111

You are using your original version of the test script, not the (very slightly) corrected version I committed (I committed the original, then committed the changes separately).

The first row shows 'MDFlatButtons from KV'. These buttons have no background so it matches the Material Design spec, as far as I can tell, not to show any background when disabled.

In the second row in this image, although it says 'MDFlatButton from Python' they are actually 'MDRaisedButton from Python' (you can see that the first button is actually titled 'MDRaisedButton'; the other two are mislabelled). Consequently, having a background when disabled is correct for this button.

These two rows aren't normally next to each other though - I assume you cropped out the bit in the middle for comparison? If not, my explanation could be wrong.

As requested, I will remove this script - however, it seems like a good test script so if it is useful then why not put it in the repository? Otherwise, there are virtually no tests which means people contributing a) don't have tests to run and b) don't know what sort of tests are desired.

@gbm001
Copy link
Contributor Author

gbm001 commented Dec 9, 2021

That said, there is something weird about the shadow on the disabled MDRaisedButton (there shouldn't be one, for a start?).

@gbm001
Copy link
Contributor Author

gbm001 commented Dec 9, 2021

That said, there is something weird about the shadow on the disabled MDRaisedButton (there shouldn't be one, for a start?).

There was an issue with icons created in Python which were initially disabled; they had their elevation shadows displayed instead of hidden.
This is now fixed (by calling on_disabled in init in the button elevation mix-in).

@gbm001
Copy link
Contributor Author

gbm001 commented Dec 9, 2021

Currently the MDRaisedButton doesn't show any elevation when not pressed, contrary to the MD spec (although I don't think it did before, either). Possibly best as a separate PR?

Also no M3 buttons are currently available, but that should definitely be the subject of a separate PR...

@HeaTTheatR
Copy link
Member

Also no M3 buttons are currently available, but that should definitely be the subject of a separate PR...

https://kivymd.readthedocs.io/en/latest/components/button/#material-design-style-3

@gbm001
Copy link
Contributor Author

gbm001 commented Dec 9, 2021

Also no M3 buttons are currently available, but that should definitely be the subject of a separate PR...

https://kivymd.readthedocs.io/en/latest/components/button/#material-design-style-3

Apologies - AFAIK no M3 buttons are available except for the Floating Action Buttons :)
That functionality should be preserved, but there are no tests so I should write some.

@gbm001
Copy link
Contributor Author

gbm001 commented Dec 9, 2021

What sort of tests do you want, if not the script you provided?

@HeaTTheatR
Copy link
Member

@gbm001

Run Kitchen Sink demo:

AttributeError: 'ThemeManager' object has no attribute 'disabled_primary_color'

@HeaTTheatR
Copy link
Member

@gbm001

Format all files and run tests:

pre-commit run --all-files

https://github.com/kivymd/KivyMD#setup-environment

@gbm001
Copy link
Contributor Author

gbm001 commented Dec 13, 2021

@gbm001 Also it is not clear to me why the theme_text_color_options attribute is defined as a global variable?

Снимок экрана 2021-12-11 в 13 45 12

That list appears four times in the code (theme_text_style, _theme_text_style, theme_icon_style, _theme_icon_style). It seemed better to put it in one place than to have four repeats of the same thing.

It would be better to make the list a constant (with ALL_CAPS naming) in label/label.py even though it is only used once there, so it can be imported into this and used directly. Otherwise, if the list ever changes then you would have to chase down five different versions of it. Would you prefer this?

@gbm001
Copy link
Contributor Author

gbm001 commented Dec 13, 2021

@gbm001

Run Kitchen Sink demo:

AttributeError: 'ThemeManager' object has no attribute 'disabled_primary_color'

Weird - Kitchen Sink works fine for me; I've been using it for testing. 'disabled_primary_color' has been added to theming.py; that and adding the opposite colour version are the only changes made to KivyMD code other than buttons.py/kv.

@HeaTTheatR
Copy link
Member

@gbm001 Also it is not clear to me why the theme_text_color_options attribute is defined as a global variable?
Снимок экрана 2021-12-11 в 13 45 12

That list appears four times in the code (theme_text_style, _theme_text_style, theme_icon_style, _theme_icon_style). It seemed better to put it in one place than to have four repeats of the same thing.

It would be better to make the list a constant (with ALL_CAPS naming) in label/label.py even though it is only used once there, so it can be imported into this and used directly. Otherwise, if the list ever changes then you would have to chase down five different versions of it. Would you prefer this?

This attribute must be in the theming.py module in ThemeManager class

@gbm001
Copy link
Contributor Author

gbm001 commented Dec 13, 2021

@gbm001

PEP8

kivymd/uix/button/button.py:

Снимок экрана 2021-12-11 в 13 39 12 Снимок экрана 2021-12-11 в 13 39 30
[etc]

I'm not sure which bit of PEP8 this code isn't complying with? Do you mean PEP257 (Docstring conventions)?

I know I've done one-line docstrings as:
"""
Docstring line
"""

which isn't strictly correct by PEP257 (should just on one line), but I think you already stated that was the correct style for this project (I know you are using an automatic documentation generator so whatever works best/looks best/you want is fine).

@HeaTTheatR
Copy link
Member

@gbm001
Run Kitchen Sink demo:
AttributeError: 'ThemeManager' object has no attribute 'disabled_primary_color'

Weird - Kitchen Sink works fine for me; I've been using it for testing. 'disabled_primary_color' has been added to theming.py; that and adding the opposite colour version are the only changes made to KivyMD code other than buttons.py/kv.

No. The buttons don't work with the above error.

@HeaTTheatR
Copy link
Member

@gbm001

PEP8

kivymd/uix/button/button.py:
Снимок экрана 2021-12-11 в 13 39 12 Снимок экрана 2021-12-11 в 13 39 30
[etc]

I'm not sure which bit of PEP8 this code isn't complying with? Do you mean PEP257 (Docstring conventions)?

I know I've done one-line docstrings as: """ Docstring line """

which isn't strictly correct by PEP257 (should just on one line), but I think you already stated that was the correct style for this project (I know you are using an automatic documentation generator so whatever works best/looks best/you want is fine).

Wrong

def method(self):
    """Doc"""
    code

Right

def method(self):
    """Doc"""

    code

@gbm001
Copy link
Contributor Author

gbm001 commented Dec 13, 2021

@gbm001

PEP8

kivymd/uix/button/button.py:
Снимок экрана 2021-12-11 в 13 39 12 Снимок экрана 2021-12-11 в 13 39 30
[etc]

I'm not sure which bit of PEP8 this code isn't complying with? Do you mean PEP257 (Docstring conventions)?
I know I've done one-line docstrings as: """ Docstring line """
which isn't strictly correct by PEP257 (should just on one line), but I think you already stated that was the correct style for this project (I know you are using an automatic documentation generator so whatever works best/looks best/you want is fine).

Wrong

def method(self):
    """Doc"""
    code

Right

def method(self):
    """Doc"""

    code

That's not a requirement of PEP8 or PEP257 (except for class docstrings), but perfectly happy to do that :)

@HeaTTheatR
Copy link
Member

That's not a requirement of PEP8 or PEP257 (except for class docstrings), but perfectly happy to do that :)

It looks terrible and the code shouldn't be written in this style.

@gbm001
Copy link
Contributor Author

gbm001 commented Dec 13, 2021

@gbm001

Format all files and run tests:

pre-commit run --all-files

https://github.com/kivymd/KivyMD#setup-environment

I had to also install pytest-cov, pytest-timeout and pyinstaller on top of the requirements stated in the link, but I got all the tests to pass (with 5 warnings not related to buttons.py) :)

@gbm001
Copy link
Contributor Author

gbm001 commented Dec 13, 2021

@gbm001
Run Kitchen Sink demo:
AttributeError: 'ThemeManager' object has no attribute 'disabled_primary_color'

Weird - Kitchen Sink works fine for me; I've been using it for testing. 'disabled_primary_color' has been added to theming.py; that and adding the opposite colour version are the only changes made to KivyMD code other than buttons.py/kv.

No. The buttons don't work with the above error.

I haven't made any changes to the Kitchen Sink code, and it definitely works for me. How are you doing the testing on your end - have you cloned the fork and made a virtual environment, or just copied in the buttons.py and theming.py files? In this PR ThemingManager definitely has a 'disabled_primary_color' attribute.

@HeaTTheatR
Copy link
Member

I haven't made any changes to the Kitchen Sink code, and it definitely works for me. How are you doing the testing on your end - have you cloned the fork and made a virtual environment, or just copied in the buttons.py and theming.py files? In this PR ThemingManager definitely has a 'disabled_primary_color' attribute.

The demo application has nothing to do with it. Error in your changes.

@gbm001
Copy link
Contributor Author

gbm001 commented Dec 13, 2021

@gbm001 Also it is not clear to me why the theme_text_color_options attribute is defined as a global variable?
Снимок экрана 2021-12-11 в 13 45 12

That list appears four times in the code (theme_text_style, _theme_text_style, theme_icon_style, _theme_icon_style). It seemed better to put it in one place than to have four repeats of the same thing.
It would be better to make the list a constant (with ALL_CAPS naming) in label/label.py even though it is only used once there, so it can be imported into this and used directly. Otherwise, if the list ever changes then you would have to chase down five different versions of it. Would you prefer this?

This attribute must be in the theming.py module in ThemeManager class

I'm not sure how that could work?

In label.py, you have code:

class MDLabel(ThemableBehavior, Label, MDAdaptiveWidget):

    [stuff...]

    theme_text_color = OptionProperty(
        "Primary",
        allownone=True,
        options=[
            "Primary",
            "Secondary",
            "Hint",
            "Error",
            "Custom",
            "ContrastParentBackground",
        ],
    )

If the list ["Primary", "Secondary", "Hint", "Error", "Custom", "ContrastParentBackground"] was an attribute on ThemeManager called theme_text_color_options, the only way to use it in this code would be:

from kivymd.theming import ThemeManager

class MDLabel(ThemableBehavior, Label, MDAdaptiveWidget):

    [stuff...]

    theme_text_color = OptionProperty(
        "Primary",
        allownone=True,
        options=ThemeManager.theme_text_color_options
    )

which seems a bit fragile as it directly references ThemeManager by name to access a class-level attribute. You can't use self.theme_cls here (class-level declarations).

The declarations in buttons.py for theme_text_color, _theme_text_color, theme_icon_color, _theme_icon_color are equivalent.

The available list of options is just a constant that is only relevant to text labels; would it not make sense to just declare the constant at the top of label.py?

@HeaTTheatR
Copy link
Member

HeaTTheatR commented Dec 13, 2021

If the list ["Primary", "Secondary", "Hint", "Error", "Custom", "ContrastParentBackground"] was an attribute on ThemeManager called theme_text_color_options, the only way to use it in this code would be:

Attribute

     theme_text_color = OptionProperty (
         ...
     )

in this case, it should be moved to the ThemeManager class

@gbm001
Copy link
Contributor Author

gbm001 commented Dec 13, 2021

If the list ["Primary", "Secondary", "Hint", "Error", "Custom", "ContrastParentBackground"] was an attribute on ThemeManager called theme_text_color_options, the only way to use it in this code would be:

Attribute

     theme_text_color = OptionProperty (
         ...
     )

in this case, it should be moved to the ThemeManager class

If you moved the OptionProperty theme_text_color of MDLabel to the ThemeManager class, then every label would have the same theme_text_color? It's just the list of allowable options, which is a constant set at the time of class creation, that I wouldn't want to have repeated 5 times in the code (4 times in buttons.py, 1 time in label.py).

@gbm001
Copy link
Contributor Author

gbm001 commented Dec 13, 2021

I haven't made any changes to the Kitchen Sink code, and it definitely works for me. How are you doing the testing on your end - have you cloned the fork and made a virtual environment, or just copied in the buttons.py and theming.py files? In this PR ThemingManager definitely has a 'disabled_primary_color' attribute.

The demo application has nothing to do with it. Error in your changes.

For the sake of completeness, I have just carried out the following steps.

git clone git@github.com:gbm001/KivyMD.git
cd KivyMD
git checkout gbm001-refactor-buttons
virtualenv env
source env/bin/activate
pip install kivy
pip install -e .
python demos/kitchen_sink/main.py

This works. The Kitchen Sink demo appears, looking at the buttons page works, clicking the buttons produces the right animations... what am I doing wrong?

@HeaTTheatR
Copy link
Member

This works. The Kitchen Sink demo appears, looking at the buttons page works, clicking the buttons produces the right animations... what am I doing wrong?

Perhaps I forgot to enable changes in the theming.py module

@gbm001 gbm001 marked this pull request as ready for review December 13, 2021 14:32
@gbm001
Copy link
Contributor Author

gbm001 commented Dec 13, 2021

I think I'd be happy to see this merged now; there are a few things that are a bit imperfect but I'm not sure what the best way to go is and they can be fixed up in the future when M3 buttons are implemented (which might dictate the design a bit). So I've marked this 'ready for review' now and ready for more comments/improvements :)

@HeaTTheatR
Copy link
Member

I think I'd be happy to see this merged now; there are a few things that are a bit imperfect but I'm not sure what the best way to go is and they can be fixed up in the future when M3 buttons are implemented (which might dictate the design a bit). So I've marked this 'ready for review' now and ready for more comments/improvements :)

Fix all PEP8...

@gbm001
Copy link
Contributor Author

gbm001 commented Dec 13, 2021

I think I'd be happy to see this merged now; there are a few things that are a bit imperfect but I'm not sure what the best way to go is and they can be fixed up in the future when M3 buttons are implemented (which might dictate the design a bit). So I've marked this 'ready for review' now and ready for more comments/improvements :)

Fix all PEP8...

I think that's already done now?

(when you say PEP8, I'm taking that to mean 'PEP8 general style guidelines + PEP257 which is the main Docstring style guide + consistency with the rest of the project + the automated documentation markers'). Standard PEP 257 does not put a blank line between a method docstring and the code BUT it doesn't say you can't either, so I have added all these blank lines as requested. PEP 257 does say you should have a blank line after a class docstring, which I had got wrong a few times so I added those as well).

pre-commit tidied up a few other things (Black is very opinionated by design above and beyond PEP8), and I think I've made all the other changes you suggested?

@gbm001
Copy link
Contributor Author

gbm001 commented Dec 13, 2021

I should also add that as a big set of changes, it's quite possible that there will be some bugs that crop up but I am happy to fix them as they arise and do continuing maintenance on this part.

(also I have to completely rewrite the mess of subclassed buttons I have in my own app, which hopefully be a lot cleaner now! :) )

kivymd/uix/button/button.py Outdated Show resolved Hide resolved
kivymd/uix/button/button.py Outdated Show resolved Hide resolved
@HeaTTheatR HeaTTheatR merged commit 4be03e7 into kivymd:master Dec 13, 2021
@DaytonaJohn
Copy link

Just a suggestion to add a test to your test suite. The current (0.104.2) kivymd has code that explicit sets md_bg_color to [0.0, 0.0, 0.0, 0.0] in the on_disabled() code of BaseFlatButton. That code results in the user set value of md_bg_color being lost if a button that was initially enabled, is disabled and then re-enabled. Tests that cycle through enabled-disabled-enabled would catch that bug.

@HeaTTheatR
Copy link
Member

@DaytonaJohn We do not support version 0.104.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants