Skip to content

Use "Set value" as title for QInputDialog of some widgets. - #4063

Merged
Wallacoloo merged 1 commit into
LMMS:masterfrom
husamalhomsi:Widgets
Mar 14, 2018
Merged

Use "Set value" as title for QInputDialog of some widgets.#4063
Wallacoloo merged 1 commit into
LMMS:masterfrom
husamalhomsi:Widgets

Conversation

@husamalhomsi

@husamalhomsi husamalhomsi commented Dec 21, 2017

Copy link
Copy Markdown
Member

These widgets are the Fader, Knob and LcdSpinBox.
The QInputDialog shows when a widget from the above is double-clicked.
I propose this change because the current title windowTitle() usually doesn't have a specified value and defaults to lmms:
Before
With this change, it will always default to Set value:
After

These widgets are the Fader, Knob and LcdSpinBox.
@tresf

tresf commented Dec 21, 2017

Copy link
Copy Markdown
Member

This looks good. In regards to the way we're handling this, I feel we may be able to consolidate the redundant strings. @liushuyu any thoughts?

@Wallacoloo

Copy link
Copy Markdown
Member
  1. Given how both LcdSpinBox and Fader already duplicate the tr( "Please enter a new value between %1 and %2:" ) string when calling QInputDialog, I think it's not that much worse duplicating the other strings for now.

  2. The easiest way to consolidate these calls is to create a new method getInput on the ModelView class, which creates and populates a QInputDialog. I'll try to look into it this week.

@Wallacoloo Wallacoloo self-assigned this Mar 13, 2018
@Wallacoloo
Wallacoloo merged commit 7adad47 into LMMS:master Mar 14, 2018
@Wallacoloo

Copy link
Copy Markdown
Member

I went ahead and merged this as per my point 1 above. I still plan to consolidate these - but when I took a first pass at it today it turned out there's some cleanup around ModelViews/Models required to make the consolidation work well, so it's going to have to happen incrementally.

@husamalhomsi
husamalhomsi deleted the Widgets branch March 14, 2018 16:31
@husamalhomsi
husamalhomsi restored the Widgets branch March 14, 2018 16:31
@husamalhomsi
husamalhomsi deleted the Widgets branch June 10, 2019 23:16
sdasda7777 pushed a commit to sdasda7777/lmms that referenced this pull request Jun 28, 2022
Use "Set value" as title for QInputDialog of some widgets.
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