X Tutup
Skip to content

Add a tag tab + static tag list#38197

Merged
mmattel merged 1 commit intomasterfrom
issues/4294
Dec 12, 2020
Merged

Add a tag tab + static tag list#38197
mmattel merged 1 commit intomasterfrom
issues/4294

Conversation

@AlexAndBear
Copy link

@AlexAndBear AlexAndBear commented Dec 8, 2020

Description

  1. Introduce a new tab, where the tag input field will be displayed.
    1.1 Keyboard navigation in the input field's dropdown works now proper, in the past the user needed to hover over on item in the dropdown before the arrow keys could be used

  2. Replace the tag input field in the file list by a read only tag list.
    2.1 Improving tag appearance to clarify that these are tags and no shares.
    2.2 Clicking on a tag opens the tag tab.
    2.3 Tag tab and tag list are in sync, means that editing (select, unselect, rename, remove) tags will appear in the tag list immediately.

Related Issue

Screenshots (if appropriate):

Screenshot_20201211_123331

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Database schema changes (next release will require increase of minor version instead of patch)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Technical debt
  • Tests only (no source changes)

Checklist:

@AlexAndBear AlexAndBear force-pushed the issues/4294 branch 3 times, most recently from 7447355 to 85704f0 Compare December 8, 2020 15:43
@AlexAndBear AlexAndBear marked this pull request as ready for review December 9, 2020 12:53
@AlexAndBear AlexAndBear changed the title [WIP] Issues/4294 Issues/4294 Dec 9, 2020
@AlexAndBear AlexAndBear changed the title Issues/4294 Add a tag tab + static tag list Dec 9, 2020
@phil-davis
Copy link
Contributor

The drone agents + docker "have a bad day" sometimes. https://drone.owncloud.com/owncloud/core/27900/152/8

Error response from daemon: No such image: owncloudci/php:7.4

That is rubbish - the docker image should be pulled from docker if the agent does not already have it.

I restarted drone CI.

@AlexAndBear
Copy link
Author

The drone agents + docker "have a bad day" sometimes. https://drone.owncloud.com/owncloud/core/27900/152/8

Error response from daemon: No such image: owncloudci/php:7.4

That is rubbish - the docker image should be pulled from docker if the agent does not already have it.

I restarted drone CI.

I think today drone is not willing to work properly

+ php occ security:certificates:import /drone/server.crt
--
188 | certificate not found

@mmattel
Copy link
Contributor

mmattel commented Dec 10, 2020

Some test have not started, services not ready...

+ wait-for-it -t 600 fsweb.test.owncloud.com:445
--
59 | services aren't ready in 10m0s

@AlexAndBear
Copy link
Author

Some test have not started, services not ready...

+ wait-for-it -t 600 fsweb.test.owncloud.com:445
--
59 | services aren't ready in 10m0s

Yes, drone ist really frustrating today....

@phil-davis
Copy link
Contributor

fsweb.test.owncloud.com was down. The sysadmins have got it up again. I restarted drone CI.

@AlexAndBear
Copy link
Author

fsweb.test.owncloud.com was down. The sysadmins have got it up again. I restarted drone CI.

Thank you, checks passed now

@mmattel
Copy link
Contributor

mmattel commented Dec 11, 2020

@janackermann
I have tried it and it works great.
The following came to my mind when trying an eMail address as tag:

  • Yes, you get the info "This looks like an eMail...."
    But what do I do with that info, which consequences arise?
    Pressing enter adds the tag, what do I have to fear if I have done so?
  • This looks like an email address or federated cloud ID. Are you sure you want to use this tag name?
    needs improvement, because you are checking the @ character. -->
    This looks like you start typing an email address or federated cloud ID. Are you sure you want to use this kind tag name?
  • When you start typing the tag with @ like @JanAckermann, this does NOT look like an eMail address. Where I am not sure if that would be a valid start for a federated share. A federated share, as far I remember, starts with username@instance-url. Maybe you want to differenciate this in the text shown (text = case dependent). Saying that, I see that there are three cases. @ (alone), name@ and @name ...
  • It would be great, if the editable tag list would open not only when clicking the mouse into the horizontal tag list, but also on the tag itself.

@AlexAndBear
Copy link
Author

@mmattel Thank you for your Input!

Yes, you get the info "This looks like an eMail...."
But what do I do with that info, which consequences arise?

@pmaier1 @micbar I think we could really get rid of this message, from now it is so clear that these are tags, what do you think?

It would be great, if the editable tag list would open not only when clicking the mouse into the horizontal tag list, but also on the tag itself.
I don't get it, could you explain maybe with pictures :) ?

@micbar
Copy link
Contributor

micbar commented Dec 11, 2020

@mmattel

Yes, you get the info "This looks like an eMail...."
But what do I do with that info, which consequences arise?

The problem which started this PR is, that many users enter a username in the tags field and think that they have shared the file. The instance we are talking about has a lot of federated shares.

But i would suggest to rather remove the message at all. It was an idea to help the users, but seems to be not as helpful as we thought.

@janackermann @JammingBen agree?

@JammingBen
Copy link
Contributor

The problem which started this PR is, that many users enter a username in the tags field and think that they have shared the file. The instance we are talking about has a lot of federated shares.

But i would suggest to rather remove the message at all. It was an idea to help the users, but seems to be not as helpful as we thought.

@janackermann @JammingBen agree?

I totally agree. In the future, users have to click either on the "tags"-tab, or on the tags directly which will also open the "tags"-tab. So IMO this makes in pretty clear that we are dealing with tags here, not with email addresses.

@AlexAndBear
Copy link
Author

Message removed

@mmattel
Copy link
Contributor

mmattel commented Dec 11, 2020

@janackermann
It would be great, if it would be possible to edit a particular tag by just clicking on it versus clicking in a free part of the total (horizontal) list to open the full editable list of tags and then pick the one to change.
At least, clicking on any item in the horizontal list to open the editable list (compared to now where you need to click on the right free space of the horizonal list)

@AlexAndBear
Copy link
Author

AlexAndBear commented Dec 11, 2020

@janackermann
It would be great, if it would be possible to edit a particular tag by just clicking on it versus clicking in a free part of the total (horizontal) list to open the full editable list of tags and then pick the one to change.
At least, clicking on any item in the horizontal list to open the editable list (compared to now where you need to click on the right free space of the horizonal list)

@mmattel Unfortunately Select2 3.5 stops the propagation of most events out of the container so I am not able to catch the events when a tag is just clicked, with Select2 v4, this is no issue any more but needs a bunch of refactoring. I think this would blow up the frame of this PR.

@mmattel
Copy link
Contributor

mmattel commented Dec 11, 2020

Select2 v4 is out of reach of this PR, totally agree. You may file another PR for that...

And CI is making us crazy...
What the h**l is going on atm?

Error: connect ETIMEDOUT 52.216.243.76:443
--
28 | at TCPConnectWrap.afterConnect [as oncomplete] (net.js:1191:14)

@AlexAndBear
Copy link
Author

Select2 v4 is out of reach of this PR, totally agree. You may file another PR for that...

And CI is making us crazy...
What the h**l is going on atm?

Error: connect ETIMEDOUT 52.216.243.76:443
--
28 | at TCPConnectWrap.afterConnect [as oncomplete] (net.js:1191:14)

Basically the the lisence of the windows server has been expired, so the server shuts down automatically.
Cristian already takes care about this problem

@mmattel
Copy link
Contributor

mmattel commented Dec 11, 2020

🤦‍♂️
When do you think it is up again?

@AlexAndBear
Copy link
Author

When do you think it is up again?

I have no clue 🧐

@sonarqubecloud
Copy link

Kudos, SonarCloud Quality Gate passed!

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

No Coverage information No Coverage information
No Duplication information No Duplication information

Copy link
Contributor

@phil-davis phil-davis left a comment

Choose a reason for hiding this comment

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

Minor comments on the changelog

@owncloud owncloud deleted a comment from update-docs bot Dec 12, 2020
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.

5 participants

X Tutup