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

Helpful error #302

Merged
merged 5 commits into from Nov 11, 2021
Merged

Helpful error #302

merged 5 commits into from Nov 11, 2021

Conversation

13r0ck
Copy link

@13r0ck 13r0ck commented Nov 10, 2021

Closes #296

jackpot51
jackpot51 previously approved these changes Nov 10, 2021
@jackpot51 jackpot51 requested a review from a team November 10, 2021 23:08
@jacobgkau
Copy link
Member

It looks like there's a stray space before the second bullet point, so the bullets aren't in line:
Screenshot from 2021-11-10 16-07-43

Also, is there a reason you deviated from the messaging in #296? Looking at Try the following to resolve this error: vs. The following may resolve the issue:.

Also wondering if Make sure you're connected to the Internet. should also be a bullet point, knowing this dialog shows up when attempting to install an app offline, but it probably doesn't need to be there since the main window already has a message about it.

@13r0ck
Copy link
Author

13r0ck commented Nov 11, 2021

I changed the two points you mentioned above 👍

I don't think that a Make sure you're connected to the Internet. should be part of the message. I think an upstream PR that disables the Install button while networking isn't available solves the error before the user can create it

@jacobgkau
Copy link
Member

I think an upstream PR that disables the Install button while networking isn't available solves the error before the user can create it

I don't know if that sounds like a good idea:

  • If "networking" means an internet connection, there could conceivably be a time when a user wants to install something locally or from a networked resource without internet access, so hiding the Install button would not be desirable.
  • If "networking" does not have to include an internet connection, then the Install button would still be shown in cases where the user is connected to a network but the network's internet connection is down, and they would still end up seeing the error dialog.

I can go without the bullet point, though, since the main window already has messaging about the network connection, like I said. (The Details section also has a very clear Cannot download packages whilst offline message from PackageKit.)

@13r0ck
Copy link
Author

13r0ck commented Nov 11, 2021

I think the user interface of Pop!_Shop is to be very simple. I think the target audience is much more likely to run into a situation where they try to install an app without internet than they are to need to install an app from a local network while they don't have internet access.

Either way, It doesn't make sense to have the same error message have a Check For Updates button and a check if you are connected to the internet. The install button should either be disabled when there is no internet, or there should be a different error dialog all together.

Just for clarity, the current definition in Pop!_Shop of "no internet" is the following:

"Available" here means that the system has a default route available for at least one of IPv4 or IPv6.
It does not necessarily imply that the public Internet is reachable. 

@jacobgkau
Copy link
Member

Either way, It doesn't make sense to have the same error message have a Check For Updates button and a check if you are connected to the internet.

The app is showing the same dialog for both situations.

@13r0ck
Copy link
Author

13r0ck commented Nov 11, 2021

Yes it is. Another PR will be required to separate the two.

Either disabling the install, or create a new error dialog. A new PR is required to fix the separate networking issue at hand.

Copy link
Member

@jacobgkau jacobgkau left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Message is showing up as intended now:
Screenshot from 2021-11-11 12-16-48
Both buttons work.

@jackpot51 jackpot51 merged commit 60d6e84 into master Nov 11, 2021
@jackpot51 jackpot51 deleted the helpful-error branch November 11, 2021 19:36
@mmstick
Copy link
Member

mmstick commented Nov 11, 2021

I wonder if it'd be a good idea to have a link to where to report an issue.

@13r0ck
Copy link
Author

13r0ck commented Nov 11, 2021

@maria-komarova what do you think of mmstick's idea here? I like the idea, but I'm not sure how we can add much more without falling back into the issue Linus had with the terminal, after there is too much information, none of it is read.

@qwertychouskie
Copy link

In the original issue, I had this line as the last line in the error message:

If you still encounter this error, check out our friendly support! https://support.system76.com

@leviport
Copy link
Member

@qwertychouskie The support team is for System76 customers only, and there are many people who run Pop!_OS on non-System76 hardware, so I don't think linking to that page is desirable. We don't want to lead people to the support team only to be turned away.

@maria-komarova
Copy link

I think we can shorten the copy a bit, since our suggested action button already says Check for updates. That would help a bit with the amount of reading. But I think it is nice to have the last resort message in case the previous two solutions didn't work.
Copy could be:

Failed to install Atom.

Try the following to resolve this error:

  • Install all available updates, reboot, try again.
  • If there are other software packages available in the drop-down next to the Install button, select a different option and try again.

If you still encounter this error, please report an issue on our GitHub.

@jacobgkau
Copy link
Member

The first bullet regarding updates can definitely be shortened like that, that's a great point. We don't need to explain where the updates are located since we have a button for them in the dialog.

If there are other software packages available in the drop-down next to the Install button,

I don't know if "other software packages" is the clearest way to describe a different package type for the same application.

@maria-komarova
Copy link

maria-komarova commented Nov 12, 2021

I don't know if "other software packages" is the clearest way to describe a different package type for the same application.

Maybe not, but I can't yet think of a better way that will be Linux-beginner friendly.

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.

Pop!_Shop package install error message not helpful to average users
7 participants