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

refactor: Sharing sidebar UI redesign #50282

Merged
merged 6 commits into from
Jan 23, 2025

Conversation

nfebe
Copy link
Contributor

@nfebe nfebe commented Jan 20, 2025

Description

This is a UI refresh for the sharing sidebar.

It splits shares into two sections:

  • Internal shares - and
  • External shares

Screenshots

Before After
sharing-sidebar-before-jan25 sharing-sidebar-after-jan25

Todo

  • Confirm/check whether the deprecated projects feature (Projects rework to automatic "Related resources" #28320) can be dropped?
  • Differentiation of <SharingInput> (new component or making it configurable)
  • Styling
  • Limit list length and "show all"
  • Backend API should not handle detection of share type? (for example with mail share, in sharinginput)

Checklist

  • Tests
  • Screenshots before/after for front-end changes
  • Backports

Credits

Thanks to @thlehmann-ionos from IONOS for laying the foundations for this PR in #49653

@nfebe nfebe force-pushed the refactor/48925/sharing-sidebar-redesign branch 7 times, most recently from adce548 to 6eef604 Compare January 22, 2025 07:56
Copy link
Contributor Author

@nfebe nfebe left a comment

Choose a reason for hiding this comment

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

I backtracked a bit and adopted a minimally invasive approach that touches only the front end for now. It works without altering the main share components:

  • Public sharing link
  • Sharing details

We primarily re-adjusted the sharingTab and sharingInput components.

I took this approach so we can release what we could consider a version 1 of these enhancements and move follow-up improvements to a new ticket. This way, we can ensure updates are handled via backports.

@nfebe nfebe marked this pull request as ready for review January 22, 2025 08:08
@nfebe nfebe force-pushed the refactor/48925/sharing-sidebar-redesign branch from 6eef604 to 8f8aa98 Compare January 22, 2025 08:09
@nfebe
Copy link
Contributor Author

nfebe commented Jan 22, 2025

Suggestion changes to move to new PRs:

  • Pagination (3 per section requirement) / show all
  • Create link button for external shares (Requires touching the public sharing component)

@nfebe nfebe force-pushed the refactor/48925/sharing-sidebar-redesign branch from 8f8aa98 to acfb71a Compare January 22, 2025 08:23
Copy link
Member

@marcoambrosini marcoambrosini left a comment

Choose a reason for hiding this comment

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

Design wise from my pov:

Blocking:

  • Don't show others with access if there's no one
  • "Shares with accounts and teams" instead of "Add users and teams"
  • "Create public link" instead of "Share link"
  • No separator if there are no more blocks
  • Info tooltips text:
    Internal: Use this method to share files with individuals or teams within your organization. If the recipient already has access to the share but cannot locate it, you can send them the internal share link for easy access.
    External: Use this method to share files with individuals or organizations outside your organization. Files and folders can be shared via public share links and email addresses. You can also share to other Nextcloud accounts hosted on different instances using their federated cloud ID.
  • Make trigger of the tooltip a tertiary NcButton

Non-blocking for this pr but it would be good to follow-up before release

  • Copy internal link should go into the multi-select as an always visible option
  • Share link component should show a button if no links are present as per the mockup, but below the input field

@nfebe nfebe force-pushed the refactor/48925/sharing-sidebar-redesign branch 2 times, most recently from e66901a to 1a2e480 Compare January 22, 2025 11:22
@nfebe
Copy link
Contributor Author

nfebe commented Jan 22, 2025

@marcoambrosini But the first item (which you marked as blocking) and the last non-blocking would require more changes to ShareEntryLink component.

More than just adding a new item to list of suggestions.

Given the recent issues with that component I would say it's best to separate and merge especially after :#50260

Screenshots

No public creation link One primary link Multiple links
no-pub-links one-pub-link multiple-pub-links

@nfebe nfebe requested a review from marcoambrosini January 22, 2025 11:31
@nfebe nfebe force-pushed the refactor/48925/sharing-sidebar-redesign branch from 1a2e480 to 06daebf Compare January 22, 2025 15:42
@nfebe nfebe requested a review from artonge January 22, 2025 15:55
Copy link
Contributor

@susnux susnux left a comment

Choose a reason for hiding this comment

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

Over all this looks good but some accessibility issues spotted :accessibility:

apps/files_sharing/src/components/SharingInput.vue Outdated Show resolved Hide resolved
apps/files_sharing/src/views/SharingTab.vue Outdated Show resolved Hide resolved
apps/files_sharing/src/views/SharingTab.vue Show resolved Hide resolved
apps/files_sharing/src/views/SharingTab.vue Outdated Show resolved Hide resolved
apps/files_sharing/src/views/SharingTab.vue Outdated Show resolved Hide resolved
apps/files_sharing/src/views/SharingTab.vue Outdated Show resolved Hide resolved
apps/files_sharing/src/views/SharingTab.vue Outdated Show resolved Hide resolved
apps/files_sharing/src/views/SharingTab.vue Outdated Show resolved Hide resolved
@nfebe nfebe force-pushed the refactor/48925/sharing-sidebar-redesign branch from 06daebf to 23a8f0d Compare January 23, 2025 11:11
@nfebe nfebe requested a review from susnux January 23, 2025 11:11
@nfebe nfebe force-pushed the refactor/48925/sharing-sidebar-redesign branch from 23a8f0d to 03ca5f7 Compare January 23, 2025 11:14
@nfebe nfebe requested a review from Pytal January 23, 2025 11:26
Copy link
Contributor

@susnux susnux left a comment

Choose a reason for hiding this comment

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

still a problem with using a button for additional information

apps/files_sharing/src/views/SharingTab.vue Outdated Show resolved Hide resolved
apps/files_sharing/src/views/SharingTab.vue Outdated Show resolved Hide resolved
apps/files_sharing/src/views/SharingTab.vue Outdated Show resolved Hide resolved
apps/files_sharing/src/views/SharingTab.vue Outdated Show resolved Hide resolved
@nfebe nfebe force-pushed the refactor/48925/sharing-sidebar-redesign branch from 03ca5f7 to 87af914 Compare January 23, 2025 11:57
@nfebe nfebe force-pushed the refactor/48925/sharing-sidebar-redesign branch from 5f3eeea to 984d260 Compare January 23, 2025 14:27
@nfebe nfebe requested a review from susnux January 23, 2025 14:31
Copy link
Contributor

@susnux susnux left a comment

Choose a reason for hiding this comment

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

No accessibility issues anymore ✔️
But can be cleaned up as the description is not needed anymore (too much is also not that good for screen readers because the information is duplicated, but its no hard issue anymore).

apps/files_sharing/src/views/SharingTab.vue Outdated Show resolved Hide resolved
apps/files_sharing/src/views/SharingTab.vue Outdated Show resolved Hide resolved
apps/files_sharing/src/views/SharingTab.vue Outdated Show resolved Hide resolved
apps/files_sharing/src/views/SharingTab.vue Outdated Show resolved Hide resolved
@nfebe nfebe force-pushed the refactor/48925/sharing-sidebar-redesign branch from 5eb8714 to 2921852 Compare January 23, 2025 17:20
@nfebe nfebe requested a review from susnux January 23, 2025 17:20
Copy link
Contributor

@susnux susnux left a comment

Choose a reason for hiding this comment

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

LGTM now 🚀

@nfebe
Copy link
Contributor Author

nfebe commented Jan 23, 2025

/compile

@nfebe
Copy link
Contributor Author

nfebe commented Jan 23, 2025

/backport to stable31

@susnux susnux added enhancement design Design, UI, UX, etc. 3. to review Waiting for reviews feature: sharing and removed backport-request labels Jan 23, 2025
@susnux susnux added this to the Nextcloud 31 milestone Jan 23, 2025
@nfebe nfebe force-pushed the refactor/48925/sharing-sidebar-redesign branch from 8a725e4 to 6349f6f Compare January 23, 2025 22:31
thlehmann-ionos and others added 6 commits January 23, 2025 23:36
Primarily to move it out of the way for changes in the source location.

The feature was deprecated in version 25 (#28320), five versions ago.

Refs: #48925
Signed-off-by: nfebe <[email protected]>
Create two seperate sections for internal and external shares.

Signed-off-by: nfebe <[email protected]>
Use `Create public link` for first link share creation

Signed-off-by: nfebe <[email protected]>
Signed-off-by: nextcloud-command <[email protected]>
@nfebe nfebe force-pushed the refactor/48925/sharing-sidebar-redesign branch from 6349f6f to 7ec7a4f Compare January 23, 2025 22:36
@nfebe nfebe merged commit 53e6eee into master Jan 23, 2025
120 checks passed
@nfebe nfebe deleted the refactor/48925/sharing-sidebar-redesign branch January 23, 2025 22:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve layout of sharing sidebar
5 participants