X Tutup
Skip to content

Add styling support to Check and Radio buttons #24838

Merged
ksunden merged 4 commits intomatplotlib:mainfrom
QuLogic:button-props
Jan 23, 2023
Merged

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

Conversation

@QuLogic
Copy link
Copy Markdown
Member

@QuLogic QuLogic commented Dec 29, 2022 •
edited
Loading

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 QuLogic added this to the v3.7.0 milestone Dec 29, 2022
drammock
drammock reviewed Jan 5, 2023
View reviewed changes
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']},
Copy link
Copy Markdown

@drammock drammock Jan 5, 2023

Choose a reason for hiding this comment

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

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']},

Copy link
Copy Markdown
Member Author

@QuLogic QuLogic Jan 5, 2023

Choose a reason for hiding this comment

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

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

@drammock drammock mentioned this pull request Jan 6, 2023
Merged
QuLogic
QuLogic commented Jan 13, 2023
View reviewed changes
@ksunden
Copy link
Copy Markdown
Member

ksunden commented Jan 17, 2023

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

@QuLogic
Copy link
Copy Markdown
Member Author

QuLogic commented Jan 17, 2023

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

@QuLogic QuLogic marked this pull request as ready for review January 17, 2023 01:07
@ksunden
Copy link
Copy Markdown
Member

ksunden commented Jan 17, 2023

Probably should have a whatsnew entry to highlight the api changes

@QuLogic
Copy link
Copy Markdown
Member Author

QuLogic commented Jan 19, 2023

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

dstansby
dstansby requested changes Jan 19, 2023
View reviewed changes
Copy link
Copy Markdown
Member

@dstansby dstansby left a comment

Choose a reason for hiding this comment

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

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.

@QuLogic
Copy link
Copy Markdown
Member Author

QuLogic commented Jan 21, 2023

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 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
@ksunden @meeseeksmachine
Backport PR matplotlib#24838: Add styling support to Check and Radio …
c7db8e1 
…buttons
@meeseeksmachine meeseeksmachine mentioned this pull request Jan 23, 2023
Merged
ksunden added a commit that referenced this pull request Jan 23, 2023
@ksunden
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 QuLogic deleted the button-props branch January 23, 2023 23:09
@ksunden ksunden mentioned this pull request Feb 20, 2023
Merged
6 tasks
@ksunden ksunden mentioned this pull request Jun 20, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tacaswell tacaswell tacaswell left review comments

@ksunden ksunden ksunden left review comments

@dstansby dstansby dstansby approved these changes

+1 more reviewer

@drammock 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

@QuLogic @ksunden @tacaswell @drammock @dstansby
X Tutup