-
Notifications
You must be signed in to change notification settings - Fork 168
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
[full-ci] Add details accordion in sharing sidebar #5284
Conversation
de1fccd
to
fe0cc16
Compare
💥 Acceptance tests OCISSharingInternalUsers2 failed. The build is cancelled... |
fe0cc16
to
73a7a82
Compare
💥 Acceptance tests OCISSharingInternalUsers2 failed. The build is cancelled... |
73a7a82
to
bc71255
Compare
Removed original content in this comment and put it in the PR description ☝🏽 |
394eb09
to
6cfa6b8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some nitpicks incoming ;-) Also, in the shares section we're showing additionalInfo
whenever present. Would be good to show that here in the details as well. There are ownCloud instances out there where the display name is not sufficient for being able to uniquely identify a user. In that case the server can be configured to return additionalInfo
as well (which can be the username or the email address fwiw).
appearance="raw" | ||
class="oc-mr-xs" | ||
:aria-label="sharesLabel" | ||
@click="expandPeoplesAccordion()" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can spare 2 characters here, by just passing the callback ref.
@click="expandPeoplesAccordion()" | |
@click="expandPeoplesAccordion" |
appearance="raw" | ||
class="oc-mr-xs" | ||
:aria-label="linksLabel" | ||
@click="expandLinksAccordion()" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can spare 2 characters here, by just passing the callback ref.
@click="expandLinksAccordion()" | |
@click="expandLinksAccordion" |
v-oc-tooltip="seeVersionsLabel" | ||
appearance="raw" | ||
:aria-label="seeVersionsLabel" | ||
@click="expandVersionsAccordion()" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can spare 2 characters here, by just passing the callback ref.
@click="expandVersionsAccordion()" | |
@click="expandVersionsAccordion" |
v-oc-tooltip="seeVersionsLabel" | ||
appearance="raw" | ||
:aria-label="seeVersionsLabel" | ||
@click="expandVersionsAccordion()" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can spare 2 characters here, by just passing the callback ref.
@click="expandVersionsAccordion()" | |
@click="expandVersionsAccordion" |
<oc-img :src="highlightedFile.preview" alt="" /> | ||
</div> | ||
<div | ||
v-if="hasAnyShares && !isPublicPage" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm a big fan of not having too much logic in the template. Would be nice to have an additional showSharingInfo
computed property which has an early return if (isPublicPage) { return false }
and then checks for shares.
}, | ||
capitalizedModifiedDate() { | ||
const lastModifiedDate = this.formDateFromNow(this.highlightedFile.mdate, 'Http') | ||
return lastModifiedDate.charAt(0).toUpperCase() + lastModifiedDate.slice(1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use lodash-es
's upperFirst
:
upperFirst(lastModifiedDate)
}, | ||
hasPeopleShares() { | ||
return ( | ||
this.highlightedFile.shareTypes.includes(0) || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use the shareTypes
constants.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, checking for shareType 0
is not sufficient, is it? The people shares accordion shows other share types as well.
}, | ||
hasLinkShares() { | ||
return ( | ||
this.highlightedFile.shareTypes.includes(3) || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please use the shareTypes
constants.
) | ||
}, | ||
isCurrentUser() { | ||
return this.highlightedFile.ownerDisplayName === this.user.displayname |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please don't check the display name identifying the current user.
) | ||
}, | ||
detailsTableLabel() { | ||
return this.$gettext('Overview on the information about the selected file') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
return this.$gettext('Overview on the information about the selected file') | |
return this.$gettext('Overview of the information about the selected file') |
Results for oCISSharingPublicExpireAndRoles https://drone.owncloud.com/owncloud/web/17080/59/1 |
cf76dc2
to
9706e72
Compare
Results for oCISSharingBasic https://drone.owncloud.com/owncloud/web/17084/47/1
|
Results for oC10SharingInternalUsersRoot2 https://drone.owncloud.com/owncloud/web/17084/28/1 |
Results for oC10SharingPublicExpireAndRoles https://drone.owncloud.com/owncloud/web/17084/33/1 💥 The acceptance tests failed. Please find the screenshots inside ...
webUISharingPublicDifferentRoles-shareByPublicLinkDifferentRoles-feature-114.pngwebUISharingPublicDifferentRoles-shareByPublicLinkDifferentRoles-feature-248.pngwebUISharingPublicDifferentRoles-shareByPublicLinkDifferentRoles-feature-93.png |
Results for oC10SharingInternalGroups https://drone.owncloud.com/owncloud/web/17084/20/1 |
Results for oC10SharingPublicBasic https://drone.owncloud.com/owncloud/web/17084/31/1 |
Results for oC10SharingInternalGroupsToRoot https://drone.owncloud.com/owncloud/web/17084/21/1 💥 The acceptance tests failed. Please find the screenshots inside ...
webUISharingInternalGroupsToRoot-shareWithGroups-feature-128.pngwebUISharingInternalGroupsToRoot-shareWithGroups-feature-74.pngwebUISharingInternalGroupsToRoot-shareWithGroups-feature-95.pngwebUISharingInternalGroupsToRootEdgeCases-shareWithGroupsEdgeCases-feature-36.pngwebUISharingInternalGroupsToRootEdgeCases-shareWithGroupsEdgeCases-feature-37.png |
Results for oC10SharingInternalUsersRoot1 https://drone.owncloud.com/owncloud/web/17084/27/1 💥 The acceptance tests failed. Please find the screenshots inside ...
webUISharingInternalUsersToRoot-shareWithUsers-feature-256.pngwebUISharingInternalUsersToRoot-shareWithUsers-feature-271.pngwebUISharingInternalUsersToRoot-shareWithUsers-feature-53.pngwebUISharingInternalUsersToRoot-shareWithUsers-feature-70.pngwebUISharingInternalUsersToRoot-shareWithUsers-feature-99.png |
Results for oC10SharingAccept https://drone.owncloud.com/owncloud/web/17084/9/1 💥 The acceptance tests failed. Please find the screenshots inside ...
webUISharingAcceptSharesToRoot-acceptShares-feature-110.pngwebUISharingAcceptSharesToRoot-acceptShares-feature-131.pngwebUISharingAcceptSharesToRoot-acceptShares-feature-143.pngwebUISharingAcceptSharesToRoot-acceptShares-feature-158.pngwebUISharingAcceptSharesToRoot-acceptShares-feature-31.pngwebUISharingAcceptSharesToRoot-acceptShares-feature-315.pngwebUISharingAcceptSharesToRoot-acceptShares-feature-357.pngwebUISharingAcceptSharesToRoot-acceptShares-feature-50.pngwebUISharingAcceptSharesToRoot-acceptShares-feature-90.png |
Results for oC10Files2 https://drone.owncloud.com/owncloud/web/17084/11/1 💥 The acceptance tests failed. Please find the screenshots inside ...
webUIFilesDetails-fileDetails-feature-116.pngwebUIFilesDetails-fileDetails-feature-14.pngwebUIFilesDetails-fileDetails-feature-25.pngwebUIFilesDetails-fileDetails-feature-38.pngwebUIFilesDetails-fileDetails-feature-51.pngwebUIFilesDetails-fileDetails-feature-66.pngwebUIFilesDetails-fileDetails-feature-82.pngwebUIFilesDetails-fileDetails-feature-99.pngwebUIFilesList-fileList-feature-28.pngwebUIFilesList-fileList-feature-33.pngwebUIFilesSearch-search-feature-122.pngwebUIFilesSearch-search-feature-175.png |
Results for oC10Trashbin https://drone.owncloud.com/owncloud/web/17084/34/1 💥 The acceptance tests failed. Please find the screenshots inside ...
webUITrashbinDelete-trashbinDelete-feature-112.pngwebUITrashbinDelete-trashbinDelete-feature-41.pngwebUITrashbinFilesFolders-trashbinFilesFolders-feature-23.pngwebUITrashbinFilesFolders-trashbinFilesFolders-feature-38.pngwebUITrashbinFilesFolders-trashbinFilesFolders-feature-82.pngwebUITrashbinFilesFolders-trashbinFilesFolders-feature-97.pngwebUITrashbinRestore-trashbinRestore-feature-138.pngwebUITrashbinRestore-trashbinRestore-feature-14.pngwebUITrashbinRestore-trashbinRestore-feature-181.pngwebUITrashbinRestore-trashbinRestore-feature-222.pngwebUITrashbinRestore-trashbinRestore-feature-241.pngwebUITrashbinRestore-trashbinRestore-feature-260.pngwebUITrashbinRestore-trashbinRestore-feature-30.png |
Results for oC10IntegrationApp2 https://drone.owncloud.com/owncloud/web/17084/63/1 |
Results for oC10SharingInternalUsers https://drone.owncloud.com/owncloud/web/17084/23/1 💥 The acceptance tests failed. Please find the screenshots inside ...
webUISharingInternalUsers-shareWithUsers-feature-108.pngwebUISharingInternalUsers-shareWithUsers-feature-317.pngwebUISharingInternalUsers-shareWithUsers-feature-337.pngwebUISharingInternalUsers-shareWithUsers-feature-78.pngwebUISharingInternalUsersShareWithPage-shareWithUsers-feature-92.png |
Results for oC10CreateDelete https://drone.owncloud.com/owncloud/web/17084/7/1 💥 The acceptance tests failed. Please find the screenshots inside ...
webUICreateFilesFolders-createFile-feature-19.pngwebUICreateFilesFolders-createFolders-feature-27.pngwebUICreateFilesFolders-createFolders-feature-33.pngwebUICreateFilesFolders-createFolders-feature-40.pngwebUICreateFilesFolders-createFolders-feature-45.pngwebUIDeleteFilesFolders-deleteFilesFolders-feature-111.pngwebUIDeleteFilesFolders-deleteFilesFolders-feature-12.pngwebUIDeleteFilesFolders-deleteFilesFolders-feature-122.pngwebUIDeleteFilesFolders-deleteFilesFolders-feature-131.pngwebUIDeleteFilesFolders-deleteFilesFolders-feature-159.pngwebUIDeleteFilesFolders-deleteFilesFolders-feature-180.pngwebUIDeleteFilesFolders-deleteFilesFolders-feature-235.pngwebUIDeleteFilesFolders-deleteFilesFolders-feature-249.pngwebUIDeleteFilesFolders-deleteFilesFolders-feature-295.pngwebUIDeleteFilesFolders-deleteFilesFolders-feature-308.pngwebUIDeleteFilesFolders-deleteFilesFolders-feature-53.pngwebUIDeleteFilesFolders-deleteFilesFolders-feature-54.pngwebUIDeleteFilesFolders-deleteFilesFolders-feature-55.pngwebUIDeleteFilesFolders-deleteFilesFolders-feature-56.png |
Results for oC10IntegrationApp1 https://drone.owncloud.com/owncloud/web/17129/62/1 |
Results for oC10SharingPublicBasic https://drone.owncloud.com/owncloud/web/17129/31/1 |
Results for oC10Files1 https://drone.owncloud.com/owncloud/web/17129/10/1 |
Results for oC10SharingInternalUsers https://drone.owncloud.com/owncloud/web/17129/23/1 |
Results for oC10Files2 https://drone.owncloud.com/owncloud/web/17129/11/1 💥 The acceptance tests failed. Please find the screenshots inside ...
webUIFilesDetails-fileDetails-feature-116.pngwebUIFilesDetails-fileDetails-feature-14.pngwebUIFilesDetails-fileDetails-feature-25.pngwebUIFilesDetails-fileDetails-feature-38.pngwebUIFilesDetails-fileDetails-feature-51.pngwebUIFilesDetails-fileDetails-feature-66.pngwebUIFilesDetails-fileDetails-feature-82.pngwebUIFilesDetails-fileDetails-feature-99.png |
Results for oCISSharingBasic https://drone.owncloud.com/owncloud/web/17131/47/1
|
Results for oC10SharingPublicExpireAndRoles https://drone.owncloud.com/owncloud/web/17131/33/1 |
Results for oC10SharingInternalUsersRoot1 https://drone.owncloud.com/owncloud/web/17131/27/1 |
Results for oC10Files2 https://drone.owncloud.com/owncloud/web/17131/11/1 |
Results for oC10SharingPublicBasic https://drone.owncloud.com/owncloud/web/17131/31/1 |
Results for oC10SharingInternalUsers https://drone.owncloud.com/owncloud/web/17131/23/1 |
Results for oC10Files2 https://drone.owncloud.com/owncloud/web/17133/11/1 |
Results for oCISSharingBasic https://drone.owncloud.com/owncloud/web/17133/47/1
|
Results for oC10IntegrationNotifications https://drone.owncloud.com/owncloud/web/17134/61/1 💥 The acceptance tests failed. Please find the screenshots inside ...
webUISharingNotifications-notificationLink-feature-18.pngwebUISharingNotifications-shareWithUsers-feature-21.pngwebUISharingNotifications-shareWithUsers-feature-32.pngwebUISharingNotifications-shareWithUsers-feature-40.pngwebUISharingNotifications-shareWithUsers-feature-53.pngwebUISharingNotificationsToRoot-notificationLink-feature-17.pngwebUISharingNotificationsToRoot-shareWithUsers-feature-19.pngwebUISharingNotificationsToRoot-shareWithUsers-feature-31.pngwebUISharingNotificationsToRoot-shareWithUsers-feature-40.pngwebUISharingNotificationsToRoot-shareWithUsers-feature-53.png |
Results for oCISSharingBasic https://drone.owncloud.com/owncloud/web/17134/47/1
|
Results for oC10IntegrationApp1 https://drone.owncloud.com/owncloud/web/17134/62/1 |
Results for oC10Basic https://drone.owncloud.com/owncloud/web/17134/6/1 💥 The acceptance tests failed. Please find the screenshots inside ...
webUILogin-adminBlocksUser-feature-20.pngwebUIPreview-imageMediaViewer-feature-141.pngwebUIPreview-imageMediaViewer-feature-159.pngwebUIPreview-imageMediaViewer-feature-84.pngwebUIPreview-imageMediaViewer-feature-91.pngwebUIPrivateLinks-accessingPrivateLinks-feature-17.pngwebUIPrivateLinks-accessingPrivateLinks-feature-25.pngwebUIPrivateLinks-accessingPrivateLinks-feature-9.pngwebUIWebdavLockProtection-delete-feature-75.pngwebUIWebdavLockProtection-delete-feature-76.pngwebUIWebdavLockProtection-move-feature-123.pngwebUIWebdavLockProtection-move-feature-124.pngwebUIWebdavLockProtection-move-feature-145.pngwebUIWebdavLockProtection-move-feature-146.pngwebUIWebdavLockProtection-upload-feature-90.pngwebUIWebdavLockProtection-upload-feature-91.png |
Please retry analysis of this Pull-Request directly on SonarCloud. |
Results for oCISSharingBasic https://drone.owncloud.com/owncloud/web/17141/47/1
|
Results for oCISSharingBasic https://drone.owncloud.com/owncloud/web/17140/47/1
|
Results for oC10Trashbin https://drone.owncloud.com/owncloud/web/17140/34/1
|
Results for oC10Basic https://drone.owncloud.com/owncloud/web/17140/6/1 💥 The acceptance tests failed. Please find the screenshots inside ...
webUILogin-adminBlocksUser-feature-20.pngwebUIPreview-imageMediaViewer-feature-141.pngwebUIPreview-imageMediaViewer-feature-159.pngwebUIPreview-imageMediaViewer-feature-84.pngwebUIPreview-imageMediaViewer-feature-91.pngwebUIPrivateLinks-accessingPrivateLinks-feature-25.pngwebUIWebdavLockProtection-delete-feature-75.pngwebUIWebdavLockProtection-delete-feature-76.pngwebUIWebdavLockProtection-move-feature-123.pngwebUIWebdavLockProtection-move-feature-124.pngwebUIWebdavLockProtection-move-feature-145.pngwebUIWebdavLockProtection-move-feature-146.pngwebUIWebdavLockProtection-upload-feature-90.pngwebUIWebdavLockProtection-upload-feature-91.png |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Description
Adds details panel to sharing sidebar, incl. preview img (if applicable)
Related Issue
Types of changes
Checklist:
Open tasks:
enabled() {}
condition is suitable for the details-item => Don't display on Trashbin routes, which is the default for non-action sidebar accordion itemsOpen issue