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

RF: Refactoring the components panel #3673

Merged
merged 11 commits into from
Mar 19, 2021
Merged

Conversation

TEParsons
Copy link
Contributor

@TEParsons TEParsons commented Mar 9, 2021

Refactor of the components panel to:

  • Make use of wx features (like WrapSizer) rather than doing things manually
  • Index components in a way which makes it much easier to dynamically show/hide them
  • Use general function stringtools.prettyname for labels rather than doing it manually
  • Generally be more stable and readable

Appearance and function are largely unaffected, just three things different:

  1. Button labels aren't wrapping yet, there's nothing about the refactor stopping this I just need to reimplement it still Done this now!
  2. Tooltips need moving to class attributes rather than module attributes, again nothing preventing this I just haven't done it yet, I've used docstrings as a placeholder This too!
  3. I've replaced the panel buttons with static labels, buttons are still in sizers accorded to category so I can add them back in but they were kind of buggy so didn't want to spend time implementing them when labels may be better anyway. Now reinstated, but without arrows.

Pulling to dev so this is non-urgent, just wanted to get this in at some point as I'm basing a standaloneRoutines branch off of this refactor to avoid conflicts.

Todd added 3 commits March 2, 2021 14:21
…unt of code and removes unused / outdated functions. Removes collapsible category buttons (possibly temporarily) as these were prone to error and not really worth the trouble, otherwise functions as before but with less code
@codecov
Copy link

codecov bot commented Mar 9, 2021

Codecov Report

Merging #3673 (d370cfd) into dev (18140ac) will increase coverage by 0.48%.
The diff coverage is 84.84%.

Impacted file tree graph

@@            Coverage Diff             @@
##              dev    #3673      +/-   ##
==========================================
+ Coverage   43.05%   43.54%   +0.48%     
==========================================
  Files         269      274       +5     
  Lines       53030    53619     +589     
  Branches     9170     9195      +25     
==========================================
+ Hits        22834    23349     +515     
- Misses      27876    27921      +45     
- Partials     2320     2349      +29     
Impacted Files Coverage Δ
psychopy/experiment/components/pump/__init__.py 9.37% <25.00%> (+1.56%) ⬆️
psychopy/experiment/components/__init__.py 43.01% <50.00%> (-6.71%) ⬇️
psychopy/app/builder/builder.py 47.74% <73.05%> (+0.46%) ⬆️
psychopy/tools/stringtools.py 94.73% <91.66%> (+57.23%) ⬆️
psychopy/experiment/components/_base.py 66.22% <100.00%> (+0.54%) ⬆️
...sychopy/experiment/components/aperture/__init__.py 83.92% <100.00%> (+0.59%) ⬆️
psychopy/experiment/components/brush/__init__.py 100.00% <100.00%> (ø)
psychopy/experiment/components/button/__init__.py 80.53% <100.00%> (ø)
...ychopy/experiment/components/cedrusBox/__init__.py 66.12% <100.00%> (ø)
psychopy/experiment/components/code/__init__.py 90.56% <100.00%> (ø)
... and 44 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 18140ac...d370cfd. Read the comment docs.

@coveralls
Copy link

coveralls commented Mar 9, 2021

Coverage Status

Coverage increased (+0.4%) to 47.298% when pulling d370cfd on TEParsons:_justRFcomponents into 18140ac on psychopy:dev.

@lgtm-com
Copy link

lgtm-com bot commented Mar 9, 2021

This pull request introduces 3 alerts and fixes 1 when merging c028f48 into 18140ac - view on LGTM.com

new alerts:

  • 2 for Unused import
  • 1 for Unused local variable

fixed alerts:

  • 1 for Unused local variable

@lgtm-com
Copy link

lgtm-com bot commented Mar 11, 2021

This pull request introduces 3 alerts and fixes 1 when merging 90c4080 into 18140ac - view on LGTM.com

new alerts:

  • 2 for Unused import
  • 1 for Unused local variable

fixed alerts:

  • 1 for Unused local variable

@TEParsons
Copy link
Contributor Author

Update: I've reinstated category buttons, but I couldn't include the ᐊ and ᐁ from before. These are unique to plate buttons, but the way we were using plate buttons was kind of hacky - they're intended for context menus, not showing/hiding sizers within a panel. They're also an absolute nightmare to style, you have to find the protected class defining their init colours and change these then call updates on them... Whereas regular buttons can just use SetForegroundColour/SetBackgroundColour.

@lgtm-com
Copy link

lgtm-com bot commented Mar 17, 2021

This pull request introduces 3 alerts and fixes 1 when merging 2053d50 into ae519ab - view on LGTM.com

new alerts:

  • 2 for Unused import
  • 1 for Unused local variable

fixed alerts:

  • 1 for Unused local variable

@peircej
Copy link
Member

peircej commented Mar 17, 2021

With your changes to the Component structure, I'm guessing that means people's custom Components will break (because they will have their icons etc in the old location?)

…ss, it will use the UnknownComponent icon rather than crashing
@TEParsons
Copy link
Contributor Author

With your changes to the Component structure, I'm guessing that means people's custom Components will break (because they will have their icons etc in the old location?)

Most recent commit addresses this - they'll need to haveiconFile, targets, tooltip and categories in the class def rather than module def for it to use these rather than the BaseComponent and BaseVisualComponent defaults, but it now won't crash :) Some may have already - before it wasn't consistent what was where

@lgtm-com
Copy link

lgtm-com bot commented Mar 19, 2021

This pull request introduces 3 alerts and fixes 1 when merging d370cfd into 349c99a - view on LGTM.com

new alerts:

  • 2 for Unused import
  • 1 for Unused local variable

fixed alerts:

  • 1 for Unused local variable

@peircej peircej merged commit c74295a into psychopy:dev Mar 19, 2021
@TEParsons TEParsons deleted the _justRFcomponents branch May 5, 2021 11:57
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.

None yet

3 participants