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

Improve UX using generic error message for unknown repo URL #111

Merged
merged 5 commits into from
Aug 24, 2021

Conversation

dgdavid
Copy link
Member

@dgdavid dgdavid commented Aug 19, 2021

Problem

When an add-on repository is disable (e.g., after finishing the installation the DVD repositories get automatically disabled), yast2 add-on shows the "Unknown URL" label as URL, which is a bit misleading.

Solution

To use a more generic text "Not found in enabled repositories".

Additionally,

  • If the add-on is not found in enabled repositories, the Repository URL is not shown anymore in the details box
  • the code to build the additional details has been refactored (just killed a bunch of Builtins)

Screenshots

Before After
Text shown before Text show after proposed changes

Copy link
Member Author

@dgdavid dgdavid left a comment

Choose a reason for hiding this comment

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

Looking at the screenshots, I found a bit weird having the

Repository URL: Unknown repository URL

In the box to show more details below the list once the repo is selected.

Should we update that label too?

@dgdavid dgdavid requested a review from lslezak August 19, 2021 16:18
@coveralls
Copy link

coveralls commented Aug 23, 2021

Coverage Status

Coverage decreased (-0.06%) to 30.187% when pulling 3b98b2a on bsc-1188635 into eae7534 on master.

@lslezak
Copy link
Member

lslezak commented Aug 23, 2021

Regarding the details in the box below, I'd omit the Repository URL value completely if we not find a repository. I think showing the reason in the table is enough.

On the other hand for the repositories we find we should also show the repository name, that's probably more interesting and shorter/easier to remember than an URL.

@dgdavid
Copy link
Member Author

dgdavid commented Aug 23, 2021

On the other hand for the repositories we find we should also show the repository name, that's probably more interesting and shorter/easier to remember than an URL.

I guess you mean in the main table, since the Repository Alias is already shown in the details. Right?

Screenshot_sle15sp3_2021-08-23_13:57:20

@lslezak
Copy link
Member

lslezak commented Aug 23, 2021

Ah, I see, then I think it is OK, we do not need to change that.

(But I still think we should also display the name because YaST basically shows the name everywhere, alias is not displayed anywhere except this place.)

@dgdavid
Copy link
Member Author

dgdavid commented Aug 23, 2021

I have removed the Repository URL from the details box when add-on is not found in enabled repositories. PR's description and screenshots updated too.

@dgdavid dgdavid marked this pull request as ready for review August 23, 2021 17:23
aliases = info.fetch("aliases", []).join(",")

details = []
details << format(_("<b>Vendor:</b> %s<br>"), vendor)
Copy link
Member Author

@dgdavid dgdavid Aug 23, 2021

Choose a reason for hiding this comment

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

I know this changes a bit the translation since former %1 is now %s but... I hope it does not hurt. If so, I can revert the last commit completely.

Copy link
Member

Choose a reason for hiding this comment

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

Unfortunately in my experience the translators quite often overlook such small differences and they just reuse the old translations. 😟

But we have the check_po_files.rb for that reason. I run it from time to time and do fixes like this.

So go ahead, I'll run the script after few days to fix the problems. 😃

Co-authored-by: Ladislav Slezák <lslezak@suse.cz>
@dgdavid dgdavid merged commit b7385e3 into master Aug 24, 2021
@dgdavid dgdavid deleted the bsc-1188635 branch August 24, 2021 09:03
@yast-bot
Copy link
Contributor

✔️ Public Jenkins job #49 successfully finished
✔️ Created OBS submit request #913966

@yast-bot
Copy link
Contributor

✔️ Internal Jenkins job #30 successfully finished
✔️ Created IBS submit request #248745

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.

None yet

4 participants