Fix #278502: Invert tours checkbox #4180
Conversation
|
Thank you. Will merge once the builds will be ready. |
|
Also proposing a few changes to the text of the tours—will push to this PR in a few minutes. |
|
Second commit added. Feedback is welcome, but these should be small non-controversial improvements. |
|
Hello, Isaac!!! You might also want to check out https://musescore.org/en/node/278495, or maybe you saw it already. Anyhow, I am making the tours a priority to get into better shape before beta, but I do want to take some care to look at the big picture, make sure we don't have too many, too few, too much inconsistency. Would love to work with you on this, so I encourage you to post your ideas to forum for further discussion. |
| @@ -427,8 +427,8 @@ void TourHandler::displayTour(Tour* tour) | |||
| mbox->setText(tourMessages[i].message); | |||
|
|
|||
| // Add "Do not show again" checkbox | |||
| QCheckBox* showToursBox = new QCheckBox(tr("Do not show me tours"), mbox); | |||
| showToursBox->setChecked(!showTours); | |||
| QCheckBox* showToursBox = new QCheckBox(tr("Show tours on startup"), mbox); | |||
There was a problem hiding this comment.
So it turns out this is a lie (just as it is in Edit / Preferences). This option doesn't just control whether they show on startup, it controls whether they continue to show at all (eg, the tour that appears when you first access the Timeline will appear, or not, based on this setting). And the startup tour won't appear after you've already seen it, regardless of the state of this option. So the more accurate working is, "Continuing showing these tours" or something to that effect.
FWIW, I am proposing we add a Help / Tours submenu, with a "Show Tours" checkbox and a "Reset Tours" command that clears the "already seen it" flag from them all. Not totally convinced we need the "Show tours" option to be exposed in Edit / Preferences any more if we do that, but if we keep it, it should be moved out of "Program Start".
There was a problem hiding this comment.
Interesting. There's something weird, though, about how you disable tours and click "Next" and it will continue to take you to the next one instead of exiting.
There was a problem hiding this comment.
Agreed, I noticed the same thing, but then it also occurred to me it would be kind of strange if the "Next" button didn't do that. Probably what should happen is moving the default action (what happens if you press Enter) to "Close", but somehow that feels maybe awkward too. Anyhow, I say let this stand in your PR as is, I'm working on the further refinements
Hey, Marc! I actually didn't see that post, so thanks for bringing it to my attention. I just had a little bit of free time for the first time in a while, downloaded the latest nightly, and reverted to factory settings, and this was the first thing I saw that I wanted to improve. Even so, I think I'll be too busy to spend much time on this. I'm happy to step out of the way and trust your conscientious approach will lead to significant improvements. |
|
Well, I understand busy, but don't think I was asking you to step back. I definitely plan to do some organization work here and write as much as I can, but do hope to have help in the process. Hopefully the forum discussion helps me get some clarity on approach, then I'll probably create the necessary files and set up the wiring and some really basic dummy text, then maybe people can help writing specific topics. So if you want to call dibs on anything, let me know! |
IsaacWeiss
force-pushed
the
278502-tours-checkbox
branch
from
d2f7e93
to
d542727
Compare
|
I literally meant that I am happy to step back because I'm relieved that someone else will take care of it. ;-) That said, I'll try to give feedback on your own proposals. |
|
Got it :-). In that case, I recommend we close this PR and I'll just incorporate your changes into the work I already have in progress. Seems easier than trying to deal with git merge. I just submitted a PR #4193 that incorporate the change to the checkbox within the tours, also adds a command to reset tours. Meanwhile I am working on revamping the existing tours and adding new ones and will be submitting one or more PR's for that work over the coming week. |
|
@MarcSabatella I merged #4193 so this one is going to be closed, isn't it? |
|
Second commit is still relevant. |
|
I had said above I would incorporate the changes in the second commit manually into my own work, but if it's easy to merge just that now, that would be fine too. It wouldn't cause me any rebase issues if it were done today as I'm unlikely to do much with this until tomorrow. Otherwise, I will indeed just incorporate the changes manually and close this. |
|
Closed in favor of #4218. |
See https://musescore.org/en/node/278502