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

Move indicators under resource name #3617

Merged
merged 2 commits into from
Jun 25, 2020
Merged

Conversation

LukasHirt
Copy link
Contributor

@LukasHirt LukasHirt commented Jun 16, 2020

Motivation and Context

Better separation from quick actions -> probably only temporarily solution (will be tested with real users).

Screenshots (if appropriate):

image

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

@LukasHirt LukasHirt self-assigned this Jun 16, 2020
@update-docs
Copy link

update-docs bot commented Jun 16, 2020

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.

@LukasHirt LukasHirt added this to In progress in oCIS Sprint 20-12 Jun 16, 2020
@LukasHirt LukasHirt force-pushed the feature/indicators-on-second-row branch from fde5ff0 to 1313a65 Compare June 17, 2020 09:23
@LukasHirt LukasHirt marked this pull request as ready for review June 17, 2020 09:41
@LukasHirt LukasHirt moved this from In progress to To review in oCIS Sprint 20-12 Jun 17, 2020
@LukasHirt LukasHirt requested a review from kulmann June 17, 2020 09:42
@LukasHirt LukasHirt force-pushed the feature/indicators-on-second-row branch 2 times, most recently from 0731e86 to 0fc3fb6 Compare June 17, 2020 11:05
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.

Added some comments + this UX concerns:

  1. When hovering a files list row, the resource name gets highlighted (underlined). This also happens when you hover over the indicators. Which is a bit confusing, because you would expect a) the indicator to be highlighted, or b) nothing to be highlighted.
  2. The sharing status row is now part of the file item. The Shared with me, Shared with others and Trash bin file views don't support this. The Shared with others file view even has a Collaborators column, which would then replace overrule the sharing state under the resource name. We have to think this through and it widens the scope of this PR a lot.

Screenshot 2020-06-17 at 14 26 16
Screenshot 2020-06-17 at 14 26 31

apps/files/src/components/FileItem.vue Show resolved Hide resolved
changelog/unreleased/move-indicators-under-resource Outdated Show resolved Hide resolved
@LukasHirt LukasHirt force-pushed the feature/indicators-on-second-row branch from 0fc3fb6 to b692745 Compare June 17, 2020 21:21
@LukasHirt
Copy link
Contributor Author

@kulmann I've removed the underline effect when hovering over the indicators and display it only when hovering the name directly. It's done directly via styles in the component same as the sizes of the preview. I'd like to adjust the ODS component to handle two rows and then move the styles in there as well.

I've also hidden the second row in all the lists but all files and favourites as you correctly mentioned that it doesn't make much sense in there with the current implementation.

The changelog is also adjusted with your proposal 😉

I haven't unfortunately figured out what to do with the tests. I cannot reproduce the error locally either when running the tests or manually. I guess it cannot find some resources, but that doesn't really make sense to me, that it doesn't do in more scenarios and why does it pass locally.

@LukasHirt LukasHirt force-pushed the feature/indicators-on-second-row branch from b692745 to a10db53 Compare June 22, 2020 08:10
@dpakach dpakach force-pushed the feature/indicators-on-second-row branch from 6156f09 to a10db53 Compare June 23, 2020 05:00
@LukasHirt LukasHirt added this to In progress in oCIS Sprint 20-13 via automation Jun 23, 2020
@LukasHirt LukasHirt moved this from In progress to Blocked in oCIS Sprint 20-13 Jun 23, 2020
@dpakach
Copy link
Contributor

dpakach commented Jun 25, 2020

There was a bug in the test code that was scrolling through the file list and checking if the file was visible. Because of that, some elements would not be seen as visible no matter how much it scrolled.
this change (d106fb1) fixes that

@dpakach dpakach mentioned this pull request Jun 25, 2020
10 tasks
@LukasHirt
Copy link
Contributor Author

Opened an issue for labels - #3673

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.

👍

oCIS Sprint 20-13 automation moved this from Blocked to To review Jun 25, 2020
LukasHirt and others added 2 commits June 25, 2020 10:34
Adjust row selectors

Use correct row size

Use two rows only in all files list and favorites list

Truncate the resource name
@LukasHirt LukasHirt force-pushed the feature/indicators-on-second-row branch from d106fb1 to a075971 Compare June 25, 2020 08:35
@LukasHirt LukasHirt merged commit 1408ea4 into master Jun 25, 2020
oCIS Sprint 20-13 automation moved this from To review to Done Jun 25, 2020
@LukasHirt LukasHirt deleted the feature/indicators-on-second-row branch June 25, 2020 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants