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.

Sign up for GitHub

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

Jump to bottom

Add set as space img/md action #6410

Merged
merged 32 commits into from
Mar 1, 2022
Merged

Add set as space img/md action #6410

merged 32 commits into from
Mar 1, 2022

Conversation

AlexAndBear
Copy link
Contributor

@AlexAndBear AlexAndBear commented Feb 10, 2022 •
edited
Loading

Description

  • Upload space image in spaces overview
    image

  • Upload space image in space front page
    image

  • Set image file as space image in space front page (file list)
    image

  • Set text file as space description in space front page (file list)
    image

  • Change description in space front page (file list)
    image
    image

Related Issue

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests

Checklist:

  • Code changes
  • Unit tests added
  • Acceptance tests added
  • Documentation ticket raised:

Open tasks:

  • ...

@ownclouders
Copy link
Contributor

Results for oCISFiles1 https://drone.owncloud.com/owncloud/web/22636/55/1
The following scenarios passed on retry:

  • webUIDeleteFilesFolders/deleteFilesFolders.feature:253

@AlexAndBear AlexAndBear force-pushed the space-special-items-enhanchement branch from b8a7042 to 22559b3 Compare February 10, 2022 12:47
@AlexAndBear AlexAndBear force-pushed the space-special-items-enhanchement branch 2 times, most recently from b957971 to 0f60588 Compare February 14, 2022 15:36
@JammingBen JammingBen mentioned this pull request Feb 15, 2022
Merged
9 tasks
@AlexAndBear AlexAndBear force-pushed the space-special-items-enhanchement branch from e856d42 to a990545 Compare February 15, 2022 14:27
@AlexAndBear AlexAndBear force-pushed the space-special-items-enhanchement branch 3 times, most recently from 458852c to 00146b5 Compare February 17, 2022 08:34
@ownclouders
Copy link
Contributor

Results for oC10SharingAccept https://drone.owncloud.com/owncloud/web/22853/15/1
The following scenarios passed on retry:

  • webUISharingAcceptSharesToRoot/acceptShares.feature:108

@ownclouders
Copy link
Contributor

Results for oCISSharingInternal1 https://drone.owncloud.com/owncloud/web/22856/59/1
💥 The acceptance tests pipeline failed. The build has been cancelled.

@ownclouders
Copy link
Contributor

Results for oC10iPhone1 https://drone.owncloud.com/owncloud/web/22858/48/1
💥 The acceptance tests pipeline failed. The build has been cancelled.

@ownclouders
Copy link
Contributor

Results for oC10iPhone2 https://drone.owncloud.com/owncloud/web/22858/49/1
💥 The acceptance tests pipeline failed. The build has been cancelled.

@ownclouders
Copy link
Contributor

Results for oCISSharingPublic1 https://drone.owncloud.com/owncloud/web/22860/67/1
The following scenarios passed on retry:

  • webUISharingPublicExpire/shareByPublicLinkExpiringLinks.feature:11

@ownclouders
Copy link
Contributor

Results for oCISSharingPerm3 https://drone.owncloud.com/owncloud/web/22868/65/1
💥 The acceptance tests pipeline failed. The build has been cancelled.

@ownclouders
Copy link
Contributor

Results for oCISSharingInternal1 https://drone.owncloud.com/owncloud/web/22868/59/1
💥 The acceptance tests pipeline failed. The build has been cancelled.

@ownclouders
Copy link
Contributor

Results for oCISSharingInternal3 https://drone.owncloud.com/owncloud/web/22868/61/1
💥 The acceptance tests pipeline failed. The build has been cancelled.

@ownclouders
Copy link
Contributor

Results for oCISSharingAutocompletion https://drone.owncloud.com/owncloud/web/22868/62/1
💥 The acceptance tests pipeline failed. The build has been cancelled.

@tbsbdr tbsbdr mentioned this pull request Feb 18, 2022
Closed
@AlexAndBear AlexAndBear marked this pull request as ready for review February 18, 2022 09:59
@ownclouders
Copy link
Contributor

Results for oCISTrashbinUploadMoveJourney https://drone.owncloud.com/owncloud/web/22892/69/1

💥 The acceptance tests failed on retry. Please find the screenshots inside ...

webUIUserJourney-journey1_feature-L11.png

webUIUserJourney-journey1_feature-L11.png

💥 The acceptance tests pipeline failed. The build has been cancelled.

@AlexAndBear AlexAndBear force-pushed the space-special-items-enhanchement branch from 9aae261 to a8137c9 Compare February 21, 2022 11:12
@ownclouders
Copy link
Contributor

Results for oC10iPhone2 https://drone.owncloud.com/owncloud/web/22928/49/1
💥 The acceptance tests pipeline failed. The build has been cancelled.

fschade
fschade requested changes Feb 21, 2022
View reviewed changes
Copy link
Collaborator

@fschade fschade left a comment

Choose a reason for hiding this comment

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

Thanks for that nice pr!

I’ve found some smaller things:

  • in dark-mode the textarea looks a bit strange (border & text-color)
  • editDescription uses ‚subtitle‘ instead of description for gettext
  • editReadmeContent uses ‚description‘ instead of readme for gettext
  • then vs. await

it looks promising

packages/web-app-files/src/mixins/spaces/actions/editDescription.js Show resolved Hide resolved
packages/web-app-files/src/mixins/spaces/actions/editDescription.js Show resolved Hide resolved
packages/web-app-files/src/mixins/spaces/actions/editDescription.js Show resolved Hide resolved
packages/web-app-files/src/mixins/spaces/actions/editDescription.js Show resolved Hide resolved
packages/web-app-files/src/mixins/spaces/actions/editDescription.js Outdated Show resolved Hide resolved
packages/web-app-files/src/mixins/spaces/actions/editReadmeContent.js Outdated Show resolved Hide resolved
packages/web-app-files/src/mixins/spaces/actions/editReadmeContent.js Outdated Show resolved Hide resolved
packages/web-app-files/src/mixins/spaces/actions/setImage.js Outdated Show resolved Hide resolved
packages/web-app-files/src/mixins/spaces/actions/setReadme.js Outdated Show resolved Hide resolved
packages/web-app-files/src/views/spaces/Projects.vue Outdated Show resolved Hide resolved
@AlexAndBear AlexAndBear force-pushed the space-special-items-enhanchement branch from 638b64a to db8d348 Compare February 28, 2022 08:19
@ownclouders
Copy link
Contributor

Results for oCISFiles1 https://drone.owncloud.com/owncloud/web/23088/55/1
The following scenarios passed on retry:

  • webUIMarkdownEditor/markdownFile.feature:21

@ownclouders
Copy link
Contributor

Results for oCISSharingInternal2 https://drone.owncloud.com/owncloud/web/23088/60/1
The following scenarios passed on retry:

  • webUISharingInternalUsers/shareWithUsers.feature:342

@ownclouders
Copy link
Contributor

Results for oCISSharingPublic1 https://drone.owncloud.com/owncloud/web/23088/67/1
The following scenarios passed on retry:

  • webUISharingPublicBasic/publicLinkCreate.feature:190

@fschade fschade mentioned this pull request Feb 28, 2022
Merged
@kulmann kulmann mentioned this pull request Feb 28, 2022
Closed
31 tasks
@JammingBen JammingBen mentioned this pull request Feb 28, 2022
Closed
8 tasks
kulmann
kulmann requested changes Feb 28, 2022
View reviewed changes
Copy link
Member

@kulmann kulmann left a comment

Choose a reason for hiding this comment

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

Some things I found during review:

Visual:

  • button which is an overlay on the space image in the right sidebar covers a good part of the image and feels off/misplaced, see screenshot. IMO not needed there, sufficient to have it in all the context menus and in the Actions section of the right sidebar

Screenshot 2022-02-28 at 17 43 18

Git history / PR scope (because you asked for the feedback ❤️ no blockers for merging, but appreciated if you invest the time)

  • commits add opactivity and remove opacity cancel each other out - just drop the commits entirely in a rebase
  • filter driveType via API and Order spaces via backend could've been a separate PR (debatable...)
  • commit messages like don't decode id or Rebase are not self explanatory. Please add more context and ideally also reasoning, so that a quick scan of the commit list of your PR (and later on in master) gives clues about what's happening at a glance.

changelog/unreleased/enhancement-space-image-and-description-handling Outdated Show resolved Hide resolved
packages/web-app-files/src/mixins/spaces/actions/delete.js Outdated Show resolved Hide resolved
packages/web-app-files/src/mixins/spaces/actions/setImage.js Outdated Show resolved Hide resolved
packages/web-app-files/src/mixins/spaces/actions/setImage.js Show resolved Hide resolved
packages/web-app-files/src/mixins/spaces/actions/setReadme.js Outdated Show resolved Hide resolved
packages/web-app-files/src/views/spaces/Project.vue Show resolved Hide resolved
kulmann
kulmann approved these changes Mar 1, 2022
View reviewed changes
Copy link
Member

@kulmann kulmann left a comment

Choose a reason for hiding this comment

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

Awesome 🥳

@tbsbdr
Copy link
Contributor

tbsbdr commented Mar 1, 2022

quick evaluation has confirmed @kulmann s concern about the button. let's not have it on the image, but only in the 3-dots and within the actions.

@ownclouders
Copy link
Contributor

Results for oC10IntegrationApp2 https://drone.owncloud.com/owncloud/web/23122/72/1
The following scenarios passed on retry:

  • webUIUpload/upload.feature:55

@ownclouders
Copy link
Contributor

Results for oCISSharingAndUpload https://drone.owncloud.com/owncloud/web/23122/66/1
The following scenarios passed on retry:

  • webUIUpload/upload.feature:94

@sonarcloud
Copy link

sonarcloud bot commented Mar 1, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot E 1 Security Hotspot
Code Smell A 1 Code Smell

66.1% 66.1% Coverage
5.9% 5.9% Duplication

@AlexAndBear AlexAndBear merged commit 2e24276 into master Mar 1, 2022
@delete-merged-branch delete-merged-branch bot deleted the space-special-items-enhanchement branch March 1, 2022 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fschade fschade fschade approved these changes

@kulmann kulmann kulmann approved these changes

@pascalwengerter pascalwengerter Awaiting requested review from pascalwengerter

@JammingBen JammingBen Awaiting requested review from JammingBen

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add and edit space image
5 participants
@AlexAndBear @ownclouders @tbsbdr @kulmann @fschade