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

Redesign dual filter (and enable theme based styling of effect control dialogs) #3093

Closed

Conversation

simonvanderveldt
Copy link
Contributor

Fixes part of #2831

image

This uses proper layouts and CSS styling from the theme to layout and style the Dual Filter control dialog instead of hardcoded pixel values and pixmaps.

I had to change a relatively large amount of files to enable the styling using the theme/CSS for the plugin/effect control dialogs. I believe passing the parent is the correct way to do it, but please let me know if that's not the case.

P.S. The whitespace seems to a bit messed up, just noticed there's a coding conventions page, I'll fix those up after/when I get some feedback.

To enable them to use the App/parent's theme
Use layouts instead of move() commands and CSS styling sizing where possible to be useable on all
OSes and resolutions/DPIs.

The custom paintEvent is needed to make the theme work, see
http://doc.qt.io/qt-4.8/stylesheet-reference.html#qwidget-widget
@RebeccaDeField RebeccaDeField mentioned this pull request Oct 28, 2016
14 tasks
@RebeccaDeField
Copy link
Contributor

RebeccaDeField commented Oct 30, 2016

Aesthetically, I think this looks good 👍

@Mentioning some people that might have some more feedback for you...
@Umcaruje @simonvanderveldt @BaraMGB @tresf

@simonvanderveldt
Copy link
Contributor Author

Seems like either I messed someting up or it was already broken with regards to the VstEffectControls. For some reason VstEffect doesn't seem to build for me even when I run a build all. I don't see a build target for VstEffect either. Does anyone know how I can build it?

@BaraMGB
Copy link
Contributor

BaraMGB commented Nov 1, 2016

I don't think that is the right approach here. In the effect plugins we don't use the css. You should reverse all that and only redesign the plugin by code.

If you want to do it with QLayout feel free. But I made the experience that QLayout the code blow up with no benefits. Because effect plugins can't be resized by the user. Move() is just okay for this.

@Umcaruje
Copy link
Member

@simonvanderveldt the build is failing on this, plus such a change to CSS based styling is not something I think we should do for 1.2.

Can you please split this into 2 different PR's? Do a plugin redesign in inkscape like we've been doing so far to be consistent, and then have a seperate PR that does this for all plugins and then we can have this discussion there.

@simonvanderveldt
Copy link
Contributor Author

@simonvanderveldt the build is failing on this, plus such a change to CSS based styling is not something I think we should do for 1.2.

Can you please split this into 2 different PR's? Do a plugin redesign in inkscape like we've been doing so far to be consistent, and then have a seperate PR that does this for all plugins and then we can have this discussion there.

@Umcaruje thanks for the feedback. I don't have experience with Inkscape + I've just moved to a new house and have a lot of work to do there, so I guess someone else should pick this one up.

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