Skip to content
This repository has been archived by the owner on Jan 3, 2024. It is now read-only.

Fix resource icons/thumbnails only visible for clickable resources #2007

Merged
merged 2 commits into from
Mar 7, 2022

Conversation

dschmidt
Copy link
Member

@dschmidt dschmidt commented Mar 4, 2022

Description

While working on owncloud/web#6534 I was wondering why disabling thumbnails also (seemingly) disabled placeholder icons.
It turns out they are disabled for non-clickable resources, which hardly makes any sense to me.
This behavior was introduced (hopefully) by mistake in #1958 (or more specifically 2b314c9)

No tests added as the code is already tested as part of OcResource.

Related Issue

  • Fixes <issue_link>

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)
  • 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 added/updated

@update-docs
Copy link

update-docs bot commented Mar 4, 2022

Thanks for opening this pull request! The maintainers of this repository would appreciate it if you would create a changelog item based on your changes.

Copy link
Contributor

@pascalwengerter pascalwengerter left a comment

Choose a reason for hiding this comment

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

Minor change request, also adding a changelog item (and referencing the web issue) would be nice. Also interested in @lookacat's and @kulmann's feedback:)

src/components/atoms/OcResourceLink/OcResourceLink.vue Outdated Show resolved Hide resolved
@dschmidt dschmidt force-pushed the resource_show_placeholders branch from 02475a2 to fdd86b5 Compare March 7, 2022 13:01
... and fix icons/thumbnails not being shown if the resource is not clickable.
@dschmidt dschmidt force-pushed the resource_show_placeholders branch from fdd86b5 to b87ac0f Compare March 7, 2022 13:15
@sonarcloud
Copy link

sonarcloud bot commented Mar 7, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

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

70.0% 70.0% Coverage
0.0% 0.0% Duplication

Copy link
Contributor

@lookacat lookacat left a comment

Choose a reason for hiding this comment

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

LGTM 👍🏼 , seems like i didn't notice it, sorry

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.

LGTM 👍

@dschmidt
Copy link
Member Author

dschmidt commented Mar 7, 2022

@pascalwengerter I've fulfilled your request, can you un-request changes, so I can merge? :)

Copy link
Contributor

@pascalwengerter pascalwengerter left a comment

Choose a reason for hiding this comment

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

🥸

@dschmidt dschmidt merged commit 3436d6e into master Mar 7, 2022
@delete-merged-branch delete-merged-branch bot deleted the resource_show_placeholders branch March 7, 2022 15:15
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants