Add help message & icon to empty automation editor#8171
Add help message & icon to empty automation editor#8171regulus79 merged 14 commits intoLMMS:masterfrom
Conversation
|
The grey empty space around the darker middle feels a bit off. Also, it appears you are able to interact with the buttons and stuff when you first open the automation editor; they are not disabled like with the piano roll. LMMS crashed when I tried clicking on the cubic hermite curve button when no clip was open. |
I did notice that, but I couldnt figure out how to fix it.
The piano roll buttons currently aren't disabled when there isnt a midi clip open, unless theres something wrong with my build
should be a simple fix |
Aha this exists in master too. It should be fixed in |
I've added a |
src/gui/editors/PianoRoll.cpp
Outdated
| // disable all controls when no clip is selected | ||
| connect(m_editor, &PianoRoll::currentMidiClipChanged, this, [this]{ setEnabled(m_editor->hasValidMidiClip()); }); | ||
|
|
There was a problem hiding this comment.
Is this already implemented in PianoRollWindow::updateAfterMidiClipChange?
There was a problem hiding this comment.
Not anymore, I undid my changes to that function
There was a problem hiding this comment.
It appears to still be there for me(?) https://github.com/AW1534/lmms/blob/429957864d5fe19d5fbdf8f9701307ac7aec270a/src/gui/editors/PianoRoll.cpp#L5469
There was a problem hiding this comment.
oh weird, its not there on my end and i have no uncommitted/unpushed changes
There was a problem hiding this comment.
Oh i see now that you were talking about the Piano Roll and not the automation editor. It's weird that this seems to be already implemented though, as when i tested the Piano Roll before my changes, the buttons seemed to be working when no clip was selected even though they should have been disabled.
There was a problem hiding this comment.
This is a bug, the pianoroll buttons get enabled/disabled when the clip is changed but they should default to disabled. I think it should be fixed by rearranging the calls in the constructor, instead of adding this lambda connection. There's also a few more thing that can be cleaned up related to that, so I will open a separate PR for that. Could you revert this change?
There was a problem hiding this comment.
Okay, I'll revert all my Piano Roll changes
…automationEditorEmptyIcon
allejok96
left a comment
allejok96
left a comment
There was a problem hiding this comment.
I haven't bothered to check the paintEvent code, I trust you did correct whitespace changes
regulus79
left a comment
regulus79
left a comment
There was a problem hiding this comment.
I tested a build a few commits back, and it seemed to work as expected. The changes look fine to me, although I am not super familiar with all the signals in the editors, so I am not an expert here. But I couldn't find any bugs when testing.
|
Should be good for merge |
3 participants
This PR brings the changes from #8148 into the automation editor