-
-
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
adding pattern editor subwindow horizontal scaling #7409
base: master
Are you sure you want to change the base?
adding pattern editor subwindow horizontal scaling #7409
Conversation
You did an amazing job. Thank you |
This is great, now the BB editor will be better used. Do you think this can also be applied to File Explorer in the future? Because you need it. |
You could make a feature request about it and see if somebody implements it. |
@szeli1 I proposed horizontal sliding in the file browser along with another improvement in #4599 but since I didn't know anything about Github at that time they closed the issue for me for having made two suggestions in the same issue and everything was left there, no one has talked about this, I think I should have suggested different matters. |
The closed it because one suggestion was the same as an older one. |
b77214c
to
597b705
Compare
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.
Just a code review.
// if this is in the pattern editor | ||
if (m_trackView->trackContainerView()->fixedClips()) | ||
{ | ||
if (m_clipViews.size() > 0) | ||
{ | ||
auto minWidth = static_cast<size_t>(m_clipViews[0]->getClip()->length().getTicks()); | ||
setMinimumWidth(minWidth); | ||
//auto minWidth = static_cast<size_t>(m_clipViews[0]->getClip()->length().nextFullBar()); | ||
//setMinimumWidth(minWidth * m_trackView->trackContainerView()->pixelsPerBar()); | ||
} |
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.
I would personally rewrite it like this and remove the commented out code.
// if this is in the pattern editor | |
if (m_trackView->trackContainerView()->fixedClips()) | |
{ | |
if (m_clipViews.size() > 0) | |
{ | |
auto minWidth = static_cast<size_t>(m_clipViews[0]->getClip()->length().getTicks()); | |
setMinimumWidth(minWidth); | |
//auto minWidth = static_cast<size_t>(m_clipViews[0]->getClip()->length().nextFullBar()); | |
//setMinimumWidth(minWidth * m_trackView->trackContainerView()->pixelsPerBar()); | |
} | |
if (!m_trackView->trackContainerView()->fixedClips()) { return; } | |
if (m_clipViews.empty()) { return; } | |
const auto minimumWidth = static_cast<size_t>(m_clipViews.first()->getClip()->length().getTicks()); | |
setMinimumWidth(minimumWidth); |
return getTrackFixedSettingsWidth() + getTrackFixedOperationsWidth(); | ||
} | ||
|
||
const int TrackView::getTrackFixedSettingsWidth() |
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.
This function can be rewritten like this. The comments are also very unnecessary IMO: The code is clear enough that one can already tell what is happening.
const int TrackView::getTrackFixedSettingsWidth()
{
const auto isCompact = ConfigManager::inst()->value("ui", "compacttrackbuttons").toInt();
return isCompact ? DEFAULT_SETTINGS_WIDTH_COMPACT : DEFAULT_SETTINGS_WIDGET_WIDTH;
}
return DEFAULT_SETTINGS_WIDGET_WIDTH; | ||
} | ||
|
||
const int TrackView::getTrackFixedOperationsWidth() |
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.
The same with this one:
const int TrackView::getTrackFixedOperationsWidth()
{
const auto isCompact = ConfigManager::inst()->value("ui", "compacttrackbuttons").toInt();
return isCompact ? TRACK_OP_WIDTH_COMPACT : TRACK_OP_WIDTH;
}
// returns getTrackFixedSettingsWidth() + getTrackFixedOperationsWidth() | ||
static const int getTrackFixedWidth(); | ||
// returns the right compact or not compact width | ||
static const int getTrackFixedSettingsWidth(); | ||
static const int getTrackFixedOperationsWidth(); |
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.
You can make this documentation better.
Also, I removed the "Fixed" part since I don't think callers of these functions care about if its a fixed width or not. It is simply the width of the thing. If you go through with this change, you'll also have to of course update the function declarations and definitions to reflect the new name.
// returns getTrackFixedSettingsWidth() + getTrackFixedOperationsWidth() | |
static const int getTrackFixedWidth(); | |
// returns the right compact or not compact width | |
static const int getTrackFixedSettingsWidth(); | |
static const int getTrackFixedOperationsWidth(); | |
//! Returns the width of a track. | |
static const int getTrackWidth(); | |
//! Returns the width of the settings portion of a track, based on whether or not the track is compacted. | |
static const int getTrackSettingsWidth(); | |
//! Returns the fixed width of the operations portion of a track, based on whether or not the track is compacted. | |
static const int getTrackOperationsWidth(); |
Also, have you resolved @regulus79's concerns @szeli1? |
closes #7313
Made constants in
TrackView
static, replaced the configmanager "compacttrackbuttons" with static functions inTrackView
.Modified
TrackContainerView::scrollArea
to have the ability to scroll horizontally.In the past if the user increased the pattern editor's step count, the midi clips became so small they were unusable:
An other issue was if the user increased time sig to 32 (numerator) and 1 (denominator), midi clips would become invisible inside the pattern editor.
This Pull Request fixes these issues.