-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Piano Roll and Automation editor grid redesign (w/ @BaraMGB) #3062
Conversation
d4a4ea8
to
ac4c44c
Compare
That looks really great, now. |
Could it also solve this one (or include a specific fix for it)? #2832 |
To allow seperate colors for different lines? seems like a overkill to me. |
Conflicts resolved, rebased. |
@musikBear your feedback would be valuable on this. |
Could it be worth to shade horizontal lines which represent a black key? I noticed that in other programs. |
@Umcaruje Very good! Also in respect to the vertical grid-lines. I think this will be pleasant to work with |
src/gui/editors/PianoRoll.cpp
Outdated
{ | ||
if ( (barCount + leftBars) % 2 != 0 ) | ||
{ | ||
p.drawRect( x - m_currentPosition, PR_TOP_MARGIN, m_ppt, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch @BaraMGB, will fix asap. Glad you like it @musikBear. Can we close out the issue where its asked On 6 Oct 2016 14:38, "BaraMGB" notifications@github.com wrote:
|
@Umcaruje afaik, you have implemented css option for grid, and my poor vision can not be a hallmark for the design. I can fiddle with the css, and the current design you have here is clearly much better. The vertical bar-dividers are still a bit difficult to see, but yes. If noone else feel the same about bar-dividers, then this is a fine design 👍 I have tried to explain the tripple-line bar-divider WITH color, but that just to explain the idea |
@BaraMGB fixed it. |
@musikBear here's how the grid looks with your proposal. Seems like an overkill to me. |
Hmm, the 3 lines might be a bit excessive. But on the other hand, especially in the automation editor the distinction between the bar and and tick lines is pretty minimal (like in your screenshot https://cloud.githubusercontent.com/assets/6282045/19049224/d129f1c0-89a9-11e6-82cb-1d2cb7766374.png). |
If the discussion is over, this one can be merged. @musikBear @simonvanderveldt ? |
@BaraMGB I just tried this branch locally and for me the extra shading of every 2nd bar makes LMMS feel a bit "busy" (hope you know what I mean). I definitely prefer it without the shading. I do think a better visible grid is a necessity though. This goes for both the normal horizontal (notes) and vertical (ticks) lines and especially for the "major" ticks. Best example of what works really well for me/I really like is this: @Umcaruje Any clue if this will clash with darker horizontal lines for the black keys vs lighter horizontal lines for the white keys? |
I'm going to merge this in a couple of hours if there are no objections. |
I think we should address the root cause that currently makes it difficult to distinguish bars (and ticks) from each other, which IMHO is that the grid's visibility isn't good enough. This is pretty much as posted in #3019 I'm not a fan of adding extra stuff (in this case visual overhead) to fix issues whilst the root issue isn't solved. I do see that #2308 specifically requests highlighting for odd bars, but no one asked why that request was made. |
Ok then, I'll work on this more 👍 |
@Umcaruje Sorry i have been 'Off' for a week, so i have not been following your work. I have to say that i kind of like the image with the triplet line, but i also understand that you feel it does not suit your overall design. Design and 'taste' is darn difficult :) Very good work, though 👍 |
Update: Automation editor has the same grid now, and shading is more visible. Waiting for color guidelines from @RebeccaDeField and this can be tested and merged. |
@Umcaruje it is visible now, but inside the blue area in automation, i cant see it ( -It is ofcause also because of the graduated alpha, which i know is easy to disable :) |
Yes, absolutely everything is modifiable in css. You can easily change the opacity of the shading part. |
@musikBear has a good point. This may be worthy running through a colorblind test. Here's some numbers...
|
That said, I love it. :) |
After some help from @RebeccaDeField this is the current look: This PR is finally ready for merging and review. |
@tresf @musikBear @vlad1777d If this is looking good, I'll merge. |
@Umcaruje When you told me that i can work with it in CSS, im absolutely satisfied! Great job! 🎆 |
@Umcaruje , It looks great, especially violent automation line =) |
@Umcaruje , sorry that I write not in appropriate place, but I didn't found any IRC channels or something like this to ask question, (also I'd like to try adjusting colors of automation graph) Seems, it's not so clear as Piano Roll's grid. |
Here: https://discord.gg/3sc5su7 |
Well yes, but any explanation done in discord, is lost after the discord session. So the time used in discord to explain a feature, benefits exactly one person. |
@musikBear nope, Discord has persistent chat history. Besides, mentioning "consensus on Discord is ___" is writing it down for the benefit of those on github. |
@vlad1777d yeah, you can. Just edit the If you come up with a better colored grid, don't be hesitant to share, I'm always open for improvement on the gui, and your mockups made this PR happen :) |
Searchable? That would render my thought to void, but is it? ..anywitch |
@musikBear searchable, and with great filtering:
https://support.discordapp.com/hc/en-us/articles/115000468588-Using-Search
…On Sun, Mar 11, 2018, 18:07 musikBear ***@***.***> wrote:
Discord has persistent chat history
Searchable? That would render my thought to void, but is it? ..anywitch
oftopic so ..
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#3062 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AIgVmoUumJMsjCrLhp9v1I5U7RhfihxCks5tdVnNgaJpZM4KM5wg>
.
|
* Make dem grid ndasd * grid BG * fix bug in scroll behavior * debugging scrolling * Add CSS property, port to automation editor * Fix a fail * Spaces to tabs * Use fillRect rather than drawRect * Implement @vlad1777d's idea * Seperate loops and stuff * Finish up Piano Roll Grid * Cleanup * Redesign the grid for the Automation Editor * Update colors * Formatting changes * formatting changes II
Fixes #3019 and #2308
Thank you very much @BaraMGB for collaborating on this Pull Request with me and helping me out with my problems.
The grid is ligher, the background alternates shades. I also did a general CSS cleanup, fixed the indentation and stuff.
Feedback and testing welcome.