FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Add styling support to Check and Radio buttons by QuLogic · Pull Request #24838 · matplotlib/matplotlib · GitHub

  1. matplotlib
  2. matplotlib

Repository navigation

Add styling support to Check and Radio buttons - #24838

Merged
ksunden merged 4 commits into
matplotlib:mainfrom
QuLogic:button-props
Jan 23, 2023
ConversationCommits (4)ChecksFiles changed
Merged

Add styling support to Check and Radio buttons #24838
ksunden merged 4 commits into
matplotlib:mainfrom
QuLogic:button-props

Conversation

QuLogic commented Dec 29, 2022 •
edited
Loading

Copy link
Copy Markdown
Member

PR Summary

This implements arguments to override the styling of CheckButtons and RadioButtons, in a manner similar to the existing ToolHandles or Selectors. This depends on #23457 to avoid the conflicts on the arguments. I updated the examples to show what this looks like, but not the tests or documentation yet. I have two questions:

  1. Are the argument names reasonable?
  2. What to do about the label properties? The other items are put in scatter, which does have a Collection, so they accept dict with elements that are singular or lists. However, we don't have a TextCollection, so I had to make it take a dict or list of dict. This seems inconsistent, so perhaps I should attempt to implement the former input here?

Fixes #24583

PR Checklist

Documentation and Tests

  • Has pytest style unit tests (and pytest passes)
  • Documentation is sphinx and numpydoc compliant (the docs should build without error).
  • New plotting related features are documented with examples.

Release Notes

  • New features are marked with a .. versionadded:: directive in the docstring and documented in doc/users/next_whats_new/
  • API changes are marked with a .. versionchanged:: directive in the docstring and documented in doc/api/next_api_changes/
  • Release notes conform with instructions in next_whats_new/README.rst or next_api_changes/README.rst

QuLogic added this to the v3.7.0 milestone Dec 29, 2022
drammock reviewed Jan 5, 2023
View reviewed changes
Comment thread examples/widgets/check_buttons.py Outdated
actives=[l.get_visible() for l in lines_by_label.values()],
label_props=[{'color': 'k'}, {'color': 'r'}, {'color': 'g'}],
frame_props={'edgecolor': ['k', 'r', 'g']},
check_props={'facecolor': ['k', 'r', 'g']},

drammock Jan 5, 2023

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

I looked at the rendered doc and noticed that the Xs are still black, I think maybe edgecolor is what was intended here?

Suggested change
check_props={'facecolor': ['k', 'r', 'g']},
check_props={'edgecolor': ['k', 'r', 'g']},

QuLogic Jan 5, 2023

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

No, the bug is in widgets.py; I just need to work out what exactly the semantics for color/facecolor/edgecolor will be.

drammock mentioned this pull request Jan 6, 2023
Merged
QuLogic commented Jan 13, 2023
View reviewed changes
Comment thread lib/matplotlib/tests/test_widgets.py

ksunden commented Jan 17, 2023

Copy link
Copy Markdown
Member

@QuLogic I think you had indicated in alternate communication channels that this was ready for review, but it is still marked as draft.

QuLogic commented Jan 17, 2023

Copy link
Copy Markdown
Member Author

@ksunden Ah, sorry, I thought I pushed the button here; maybe an outdated browser page or something.

QuLogic marked this pull request as ready for review January 17, 2023 01:07

ksunden commented Jan 17, 2023

Copy link
Copy Markdown
Member

Probably should have a whatsnew entry to highlight the api changes

QuLogic commented Jan 19, 2023

Copy link
Copy Markdown
Member Author

Added the what's new and handling of activecolor as mentioned in call yesterday.

dstansby requested changes Jan 19, 2023
View reviewed changes

dstansby left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Choose a reason Spam Abuse Off Topic Outdated Duplicate Resolved Low Quality

Lots of suggestions and minor comments. The major change request I have is to double check this is working - I think it's worth adding a test that doesn't compare new API to new API, and looking at the what's new entry some of the styling for check buttons doesn't seem to be applied.

Comment thread doc/users/next_whats_new/widget_button_styling.rst Outdated
Comment thread doc/users/next_whats_new/widget_button_styling.rst Outdated
Comment thread doc/users/next_whats_new/widget_button_styling.rst Outdated
Comment thread lib/matplotlib/tests/test_widgets.py
Comment thread lib/matplotlib/widgets.py Outdated
Comment thread lib/matplotlib/widgets.py Outdated
Comment thread lib/matplotlib/widgets.py Outdated
Comment thread doc/users/next_whats_new/widget_button_styling.rst
Comment thread doc/users/next_whats_new/widget_button_styling.rst Outdated
Comment thread examples/widgets/check_buttons.py Outdated

QuLogic commented Jan 21, 2023

Copy link
Copy Markdown
Member Author

I modified a test image to include old and new versions, and I think should've handling your other comments in a previous push.

ksunden merged commit bf2d1dc into matplotlib:main Jan 23, 2023
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Jan 23, 2023
Backport PR matplotlib#24838: Add styling support to Check and Radio …
c7db8e1 
…buttons
meeseeksmachine mentioned this pull request Jan 23, 2023
Merged
ksunden added a commit that referenced this pull request Jan 23, 2023
Merge pull request #25065 from meeseeksmachine/auto-backport-of-pr-24…
2afc494 
…838-on-v3.7.x

Backport PR #24838 on branch v3.7.x (Add styling support to Check and Radio buttons )
QuLogic deleted the button-props branch January 23, 2023 23:09
ksunden mentioned this pull request Feb 20, 2023
Merged
6 tasks
ksunden mentioned this pull request Jun 20, 2023
Closed
This file contains hidden or 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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

tacaswell tacaswell left review comments
ksunden ksunden left review comments
dstansby dstansby approved these changes
+1 more reviewer
drammock drammock left review comments
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

New feature topic: widgets/UI

Projects

None yet

Milestone

v3.7.0

Development

Successfully merging this pull request may close these issues.

[ENH]: provide public API for styling radio buttons

5 participants

Back | FazBrowse Home | New Git URL