Relax conn dialog and make everything in app visible[CPP-574][CPP-572] #349
Conversation
|
Still can’t see your screencasts on iOS 😕 |
Can you see the ones in this pr? #334 This was recorded with Ubuntu's native recording - https://help.ubuntu.com/stable/ubuntu-help/screen-shot-record.html Not sure if what OS @john-michaelburke was on but it seems to work for me (viewing the recording from the GitHub app on iOS) |
That PR works fine, on my GitHub app it looks like this:
|
Yeah that's what this pr looks like for me as well, I think I worded my comment weirdly. I think what I'm trying to say is, if you're on Ubuntu, it seems like the native screencast feature seems to display on other platforms correctly |
|
Very strange I am able to see it via the Linux and Mac desktop chrome app. I did experience the same issues trying to watch via Android but was able to download the video and watch via VLC app on my phone. I'll record things with the Mac screen recorder next time. |
Ok, I declare a ban on whatever app you were previously using 😅 -- it doesn't play on Windows either, I was only able to play it by manually downloading the file and playing it with VLC. LICEcap: https://www.cockos.com/licecap (the GIF screen capture util I use on Windows) claims to also work on macOS. |
|
Wanted to play around with this locally, but my build environment is borked on Windows... (by carbon black, our Windows anti-virus...) -- I'm a little worried that exposing the app in an "uninitialized" state will allows users to poke at things we didn't anticipate and expose more bugs. Have you tried clicking around on things in the app to see if it behaves appropriately? I would think "appropriately" would be however we act when the app isn't connected, but in this case we're in a "never ever connected" state. |
john-michaelburke
force-pushed
the
john-michaelburke/disable-everything
branch
2 times, most recently
from
324c2c1 to
41672ca
Compare
I have played around with the application a decent amount in this "never connected" state and the various UI visible seems to work nicely. There are a few tables we may need to run back over based on Apps feedback basically should we have all tables just show a single empty row or not be visible at all. Probably could make changes per their requests in another PR though. |
john-michaelburke
force-pushed
the
john-michaelburke/disable-everything
branch
from
41672ca to
3818d15
Compare
john-michaelburke
force-pushed
the
john-michaelburke/disable-everything
branch
from
3818d15 to
f7b3514
Compare
There was a problem hiding this comment.
Looks good! Two small connection related (but unrelated to this I think) things
I started the app with my vpn off, got the pop up about the connection retrying, turned back on my vpn and got
i.e. the app started in the background but the popup didn't go away. Not sure if it's supposed to auto-dismiss but seems like we should be able to close it if the connection gets established
And then r.e. "make everything in the app visible". If we disconnect from a device we clear the settings table, but looks like we leave the rest of the data around. I feel like it should be consistent, either we clear all the data and people can poke around an empty UI, or we leave all the data in the state it was prior to the disconnect
fwiw I think the settings tab is cleared in the Drop impl on SettingsTab, removing that should make it behave like the other tabs (but we would want to change it to clear on a new connection so you don't see settings from the old device)
edit: another small pop up thing I noticed - I can't close the app if the connection refused dialog is up, but that just might be normal UX I'm not sure, but it did take me a second to figure out why a right click on the title bar didn't do anything
resources/ConnectionScreen.qml
Outdated
| id: tooltip | ||
|
|
||
| visible: connectButton.state == Constants.connection.connected && mouseArea.containsMouse | ||
| text: "Disconnect before connecting to a new device." |
There was a problem hiding this comment.
Small thing (wouldn't bother if it's not immediately obvious how to change it) but I'd expect this popup to only show when you hover over the controls themselves (maybe just the bottom set of controls?)
Currently the area shown and the area to the right of the serial/tcp radio also show it. I'm guessing the controls are spanning the entire width of the dialog, even if they aren't using all the space
There was a problem hiding this comment.
- I reduced the ToolTip area to only the area around the disabled buttons.
- I was able to have at least the Settings Table stick around after a connection is dropped (pane does not work but I think is fine for now, will probably need to disable editing when that is active)
- I disable the Settings buttons when the settings are not healthy.
- RE: Notifications need to be closed. I played around with the NoModal option and it has strange behavior, the popup goes to the middle of the screen instead of over the app and it remains open after you close the app. Per our conversation in slack with @silverjam I think we can revisit the standardization of all the various notifications seems like system popups are not the way to go.
- I have it disable the connection notification popup if a connection is established.
There was a problem hiding this comment.
Created https://swift-nav.atlassian.net/browse/CPP-589 to track the notification/snackbar idea
|
Looks good to me! |
3 participants
This is not the entire ask for CPP-572 but a portion of it. Will need to completely rewrite the connection dialog as a separate window to complete the remainder.