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 external storages settings ui: Show dedicated messages for failed to load mount points / Autofocus available for while adding external mount points #38795

Merged
merged 2 commits into from
Jun 4, 2021

Conversation

AlexAndBear
Copy link

@AlexAndBear AlexAndBear commented May 31, 2021

Description

Enhancement: Improve admin external storage settings UI

Before this PR no error notification was shown while an external mount point configuration was not able to load.
This was only indicated with a red square with a long list of external mount points, this was not handy.
Therefore an error notification will be shown with the dedicated external mount point which fails to load.

As well improved the add external mount point functionality with opening the available for select while adding an external mount point.
The select will have now a 'select all' item.
These changes prevents the admin to expose the mount point
unwanted to all users immediately.

Related Issue

Motivation and Context

How Has This Been Tested?

  • test environment:
  • test case 1:
  • test case 2:
  • ...

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:
  • Changelog item, see TEMPLATE

@AlexAndBear AlexAndBear force-pushed the improve-external-storages-settinngs-ui branch from 3d33fba to 0901cb9 Compare May 31, 2021 13:41
@AlexAndBear AlexAndBear changed the title improve-external-storages-settinngs-ui Improve external storages settings ui: Show dedicated messages for failed to load mount points / Autofocus available for while adding external mount points May 31, 2021
@AlexAndBear AlexAndBear force-pushed the improve-external-storages-settinngs-ui branch from 0901cb9 to 664e569 Compare May 31, 2021 13:56
@owncloud owncloud deleted a comment from update-docs bot May 31, 2021
@AlexAndBear AlexAndBear force-pushed the improve-external-storages-settinngs-ui branch from 664e569 to f39c627 Compare May 31, 2021 13:57
@AlexAndBear AlexAndBear marked this pull request as ready for review May 31, 2021 14:05
@jvillafanez
Copy link
Member

Double-check with multiple mount points. I think that the initial page load triggers an update status for all the mounts, so if there are several wrong mount points the notifications could be piled up.

@AlexAndBear AlexAndBear force-pushed the improve-external-storages-settinngs-ui branch from f39c627 to 3db715b Compare June 1, 2021 07:49
@AlexAndBear
Copy link
Author

@jvillafanez

Double-check with multiple mount points. I think that the initial page load triggers an update status for all the mounts, so if there are several wrong mount points the notifications could be piled up.

That's intended, customer have about 70 mount points with a large list and wants to be informed about all failing mount points without scrolling trough the list

@jvillafanez
Copy link
Member

My worry is that, if there is a connection problem and all the storages are disconnected, the admin will have 70 notifications in the web UI, and I'm not sure if the UI is ready to handle such amount. It could be fine for 3-5 errors but I think the UI will be bloated if there are more.
I don't remember a scroll in the notification area, so I suspect the area will keep expanding.

@AlexAndBear
Copy link
Author

@jvillafanez
Don't worry.

I tried it, and it looks good to me:
image

@AlexAndBear AlexAndBear force-pushed the improve-external-storages-settinngs-ui branch from 3db715b to c703a5c Compare June 1, 2021 09:50
@AlexAndBear AlexAndBear force-pushed the improve-external-storages-settinngs-ui branch from 373338f to 4ef37da Compare June 2, 2021 12:50
.htaccess Outdated Show resolved Hide resolved
@AlexAndBear AlexAndBear force-pushed the improve-external-storages-settinngs-ui branch from a69fa39 to 2e4eac9 Compare June 2, 2021 14:24
@sonarcloud
Copy link

sonarcloud bot commented Jun 2, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

Copy link
Contributor

@phil-davis phil-davis left a comment

Choose a reason for hiding this comment

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

Test changes LGTM and pass.

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.

4 participants