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

Adjust position of tutorial message (for smaller screens) #3517

Merged
merged 4 commits into from
May 27, 2020

Conversation

jonoomph
Copy link
Member

Adjust position of tutorial message for smaller screens, or when OpenShot is moved past the edge of the screen. For example:
Screenshot from 2020-05-26 18-40-02

@jonoomph
Copy link
Member Author

@ferdnyc When dragging the screen, we continue to call re_position_dialog(), which continues to call self.dock.move(QPoint(x, y)). For some reason (possibly calling move(), when Qt is already moving the widget), we get a very jumpy effect (flickering from where Qt thinks the widget should be moved, and where we want to put it). Try this PR out, and drag OpenShot off the bottom of the screen (or the right side of the screen) when the tutorial is visible. You'll see what I mean. 👍

Just curious if you know any magic way to solve this. Ultimately, I am trying to prevent the tutorials from appearing off-screen, even if it messes up our positining.

@jonoomph jonoomph requested a review from ferdnyc May 26, 2020 23:44
…glitchy).

Added ability to click tutorial anywhere to move on to the next one.
Added ability to hit ESC to hide all tutorials.
With these 2 features, partially hidden tutorial messages will be an issue no more!
@ferdnyc
Copy link
Contributor

ferdnyc commented May 27, 2020

@jonoomph

You'll see what I mean.

I'm afraid I don't, though. That's been part of the problem all along — I was aware that there are positioning issues on Windows, but they appear to be unique to Windows. And some other Linux users have reported issues with flickering and the like, but those appear to be isolated cases from people using somewhat "fringe" distros / desktops.

Here's a screencast running the code from this PR, in GNOME Shell 3.36. The current develop code behaves identically. What little glitching you do see is attributable to OBS' screen capture, and wasn't present live.

https://youtu.be/VaFXjQj25o8

Just curious if you know any magic way to solve this. Ultimately, I am trying to prevent the tutorials from appearing off-screen, even if it messes up our positining.

When this came up in the past, my take was that the simplest solution would just be to give the users control:

Also, there's an issue reported with tutorial windows being placed partially offscreen for some users (which I hadn't seen, because Gnome Shell prohibits applications from locating windows outside the visible display area). That's equally an issue with the old code, but still isn't fixed in the new code.

As a result, I think it would be best to make the tutorial popups draggable. There's no reason to prevent the user from moving the tutorial popups, if they CHOOSE to. And it would prevent more "lockout" problems where users can't advance the tutorial because the "Next Tutorial" button is inaccessible off the bottom of their screen.

I urged caution about any sort of smart placement, because (a) decent window managers already handle that, and it would suck to break things for the rest of us in an attempt to unbreak Windows' shitty positioning logic; and (b) there will always be more corner cases.

Heck, we once had an issue (can't find it now, of course — I hate you, Github issues search) where a user couldn't see the entire contents of the profiles dropdown in the Choose Profile dialog, because it was larger than his display. Normally Qt would be smart about that, but he happened to be using a dual-screen display, with the two screens stacked vertically, and the wider (top) screen happened to have positioned the dialog in the non-overlap area where the bottom screen didn't reach.. Stuff like that will always screw attempts at "intelligent" positioning.

@jonoomph
Copy link
Member Author

@ferdnyc Thanks. I decided to remove the "offscreen" logic for the tutorial (a bit too glitchy). Instead, I made the entire tutorial dialog box clickable, to move on to the next one. So, even a tiny corner can now be clicked, and the box will progress to the next one. Also, added ESC key to dismiss all tutorials.

@jonoomph
Copy link
Member Author

I've also simplified the placement logic with smarter coordinate mapping, and I'm feeling pretty good about this PR now. Feels much more polished, and less hard-coded.

jonoomph and others added 2 commits May 27, 2020 02:24
Co-authored-by: Frank Dana <ferdnyc@gmail.com>
@jonoomph
Copy link
Member Author

@ferdnyc I used GitHub to "batch" up your suggested edits and commit them. Pretty slick! I haven't done that before. 👍

@jonoomph jonoomph merged commit d783de3 into develop May 27, 2020
@jonoomph jonoomph deleted the tutorial-positioning branch May 27, 2020 07:46
@ferdnyc
Copy link
Contributor

ferdnyc commented May 27, 2020

@ferdnyc I used GitHub to "batch" up your suggested edits and commit them. Pretty slick! I haven't done that before.

@jonoomph One of my favorite features! Helps keep the git history cleaner and leaner.

They have a bunch of nice features for PRs, actually. Check this out (you'll have to scroll down a little bit): https://github.com/OpenShot/openshot-qt/pull/3463/files#diff-6b28d8a432d605b35f1930b43f84a4d6

I had no idea Actions workflow results would even be integrated into the change view — that's kind of extra-awesome. I'm hopeful Travis might possibly do the same, or at least could be taught to do it, now that we've migrated over to their Actions implementation. Just have to see if it knows how to parse GCC output.

(I keep meaning to make a test PR branch where I can introduce some errors, both non-fatal and fatal, to see how it reacts to them now.)

@ferdnyc
Copy link
Contributor

ferdnyc commented May 27, 2020

I filed #3519 with one last suggested tweak, which also has the happy effect of making the Esc-key handling global so you don't have to focus the tutorial popup first.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants