Skip to content

Conversation

@allejok96
Copy link
Contributor

When piano roll is opened for the first time in a project that has clips, it shows the help icon, but the toolbar is not grayed-out. If a clip is opened and then deleted, the toolbar gets grayed-out. This fixes it so the toolbar is disabled by default (by running setCurrentMidiClip(nullptr) after the connections are made)

Also:

  • Remove m_fileToolsButton as a class member, since all tool buttons get enabled/disabled
  • Remove redundant window naming in PianoRollWindow::setCurrentMidiClip

@regulus79
Copy link
Member

I noticed while testing that the piano roll window title doesn't update sometimes.

  1. Create a new empty project
  2. Drag in a Triple Oscillator track (or anything)
  3. Click the piano roll button to open the piano roll, which will automatically create a new clip
  4. Add some notes (or not)
  5. Right click on the clip and rename it
  6. Piano roll window title does not update

However, it appears that manually adding the clip and double-clicking to open it in the piano roll doesn't cause this issue.

@allejok96
Copy link
Contributor Author

Good find @regulus79 ! There was a difference in what signals got connected when calling PianoRoll::setCurrentMidiClip and PianoRollWindow::setCurrentMidiClip. Also, it connected to Clip::dataChanged and never disconnected on clip change, so eventually moving any clip or note would call updateAfterMidiClipChange.

I've added a separate nameChanged signal now and restored it to be like 1.2 where the instrument name was shown in the piano roll title bar.

Copy link
Member

@regulus79 regulus79 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes look fine to me.

The way the window title defaults to the track name if the clip has no name is nice!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants