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

rename tracks directly on TrackLableButton in a QLineEdit #2916

Merged
merged 1 commit into from Jul 21, 2016

Conversation

@BaraMGB
Copy link
Contributor

BaraMGB commented Jul 14, 2016

renametrack

With this one we can edit the track name directly in the track. This avoids a dialog window.

@tresf
Copy link
Member

tresf commented Jul 14, 2016

👍 👍

@BaraMGB BaraMGB force-pushed the BaraMGB:renameLineEdit branch from 3fce5fa to c7a24db Jul 14, 2016
@BaraMGB
Copy link
Contributor Author

BaraMGB commented Jul 14, 2016

I cleaned up the code a little bit.

@jasp00
Copy link
Member

jasp00 commented Jul 15, 2016

I get these errors when using compact track buttons:

QWidget::setMinimumSize: (/QLineEdit) Negative sizes (-1,0) are not possible
QWidget::setMaximumSize: (/QLineEdit) Negative sizes (-1,16777215) are not possible
@BaraMGB BaraMGB force-pushed the BaraMGB:renameLineEdit branch from c7a24db to 7b43e1a Jul 16, 2016
@BaraMGB
Copy link
Contributor Author

BaraMGB commented Jul 16, 2016

@jasp00 Thank you for revisiting this PR. I have exclude the resize in case of compact track buttons now.

{
setFixedSize( 32, 29 );
}
else
{
setFixedSize( 160, 29 );
m_renameLineEdit = new QLineEdit(this);
m_renameLineEdit->move( 30, (height() / 2) - ( m_renameLineEdit->sizeHint().height() / 2 ) );
m_renameLineEdit->setFixedWidth( this->width() - 33 );

This comment has been minimized.

@jasp00

jasp00 Jul 16, 2016 Member

Why this->width() instead of width()?

@BaraMGB BaraMGB force-pushed the BaraMGB:renameLineEdit branch from 7b43e1a to 06726f6 Jul 20, 2016
@BaraMGB
Copy link
Contributor Author

BaraMGB commented Jul 20, 2016

Why this->width() instead of width()?

Updated

@jasp00 jasp00 merged commit 7a98b3e into LMMS:master Jul 21, 2016
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@Umcaruje
Copy link
Member

Umcaruje commented Jul 24, 2016

@BaraMGB Should this also get reworked to use a disabled QLineEdit like #2918?

@BaraMGB
Copy link
Contributor Author

BaraMGB commented Jul 24, 2016

@Umcaruje no, the button is labeled with setText(). That's no QLabel or PaintEvent.

@tresf
Copy link
Member

tresf commented Jul 24, 2016

^ right. The difference here is we have a button object already that we're working from, which would make the task much more difficult. In the case of the Mixer, it was just a label, making the consolidation a no-brainer.

One could argue to abolish the button altogether, but then the button-press behavior would have to be painted, we'd essentially find ourselves making a new button-like class and the underlying logic would look very much like what's been done already.

@Umcaruje I had thought of that too when I made the recommendation on the Mixer and then realized it's a button paint (or in this case a setText(...)) and not much we could do. 👍

@midi-pascal
Copy link
Contributor

midi-pascal commented Jul 24, 2016

Just to let you know: if a track or a fx is renamed Lmms does not consider the project as modified.
You can quit with no question about saving...

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

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