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

Dual Filter Plugin Redesign #3484

Merged
merged 2 commits into from Apr 27, 2017
Merged

Dual Filter Plugin Redesign #3484

merged 2 commits into from Apr 27, 2017

Conversation

@RebeccaDeField
Copy link
Contributor

@RebeccaDeField RebeccaDeField commented Apr 3, 2017

In this pull request, I have recreated the Dual Filter plugin redesign proposed by @simonvanderveldt in #3093 as a part of our efforts here.

shot-2017-04-26_17-45-24

@Umcaruje @BaraMGB

@RebeccaDeField RebeccaDeField mentioned this pull request Apr 3, 2017
14 of 14 tasks complete
@BaraMGB
Copy link
Contributor

@BaraMGB BaraMGB commented Apr 3, 2017

That looks good to me. 👍

@RebeccaDeField
Copy link
Contributor Author

@RebeccaDeField RebeccaDeField commented Apr 4, 2017

@Umcaruje I plan on merging today if you don't have any changes 😊

@Umcaruje
Copy link
Member

@Umcaruje Umcaruje commented Apr 4, 2017

@Umcaruje I plan on merging today if you don't have any changes 😊
Sorry I was busy so I didn't have time to comment.

The box size is off by 2 pixels, and the third knob's position is off by a pixel. I'll post a fixed mockup asap.

@Umcaruje
Copy link
Member

@Umcaruje Umcaruje commented Apr 4, 2017

Actually, sorry, the knob spacing is not the same on the 2 boxes, it's either 1 or 2 pixels off. It should all be on the same value and the padding of the box should be the same on both sides.

@Umcaruje
Copy link
Member

@Umcaruje Umcaruje commented Apr 4, 2017

b1d130fa-17f6-11e7-811d-d0f5cb37d9e2
Here, I made the boxes thinner by 1px, aligned the knobs in them. I also changed the boxes' position so the spacing between the edges and the mix knob is the same. I also reduced the size of the dropdown by 1px.

These are unfortunately not code changes, just a quick mockup

@Umcaruje
Copy link
Member

@Umcaruje Umcaruje commented Apr 4, 2017

b1d130fa-17f6-11e7-811d-d0f5cb37d9e2

It looks even better to me if you deliberately move the knobs 1px off horizontal center, cause of the LED

@Umcaruje
Copy link
Member

@Umcaruje Umcaruje commented Apr 4, 2017

Also do we want a line connecting the boxes like in the stereo matrix plugin? So it's clear the mix is between the two filters?

@RebeccaDeField
Copy link
Contributor Author

@RebeccaDeField RebeccaDeField commented Apr 4, 2017

@Umcaruje Wonderful feedback, thanks! A mockup works just fine for me. I'll play with the spacing and try your idea to connect the boxes.

@Umcaruje
Copy link
Member

@Umcaruje Umcaruje commented Apr 27, 2017

I'm merging this in 12 hours if there are no objections.

@Umcaruje Umcaruje merged commit 2f51062 into LMMS:stable-1.2 Apr 27, 2017
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
PhysSong added a commit to PhysSong/lmms that referenced this pull request Jul 8, 2017
* Dual Filter

* Design Tweaks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.