Skip to content

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