cockpit-project / cockpit-files

A Featureful File Browser for Cockpit (Modernized and tested version of https://github.com/45Drives/cockpit-navigator)
GNU Lesser General Public License v2.1
46 stars 26 forks source link

Add owner column to details view #695

Closed tomasmatus closed 1 month ago

tomasmatus commented 1 month ago

Fixes: #580

Sorting is currently as follows:

I'd like to think about if we should sort current user first (as #580 mentions), I'm not sure if I like the idea but I want to check it out.

tomasmatus commented 1 month ago

current screenshot, font size is probably too big and should match font size of the "Modified" column rather than "Name" column

normal homedir screenshot: image

example screenshot when UID/GID are different: image

tomasmatus commented 1 month ago

@garrett to sum up my design related questions:

When UID/GID is a number instead of account/group name I think I should just show the number instead of doing something special. And when it comes to sorting I think we can get away with giving numbers higher priority and sort them as numbers, then local users/groups which are sorted with localeCompare.

Second question is about fontsize in the Owner column. Should it match font in Modified column or Filename column as it does now?

And lastly about "sorting current user first" I personally don't like this as to me it seems more confusing than useful but I am fine with waiting for your take on it.

allisonkarlitskaya commented 1 month ago

Writing down the result of an IRC conversation here for posterity:

We considered having separate user and group columns, and also considered always showing user:group within the same column, complete with different mixes of things like:

   oneuser : onegroup   
 otheruser : othergroup 

or

oneuser     onegroup
otheruser othergroup

but the main thrust of this is that in 99% of the directories that most users are likely to be browsing around, file ownership is the same for every file in the directory, and usually the group and username are the same. In that case, it makes things a lot visually cleaner to leave out the group.

And yes, this looks weird and becomes difficult to read in non-uniform directories, but that case is rare.

Other ideas that are more difficult to implement:

The "spooky action at a distance" stuff wouldn't necessarily need to be done via react, now that I think about it. We could have react always render the complete data but conditionally hide it via css hinged on some sort of global flag.

garrett commented 1 month ago

When UID/GID is a number instead of account/group name I think I should just show the number instead of doing something special. And when it comes to sorting I think we can get away with giving numbers higher priority and sort them as numbers, then local users/groups which are sorted with localeCompare.

What do you mean by "something special"? Do you have an example?

Second question is about fontsize in the Owner column. Should it match font in Modified column or Filename column as it does now?

All the details should use the same font family and sizing, unless they're monospace, like the permissions. And numbers should be tabular.

(Modified date/time isn't to spec, which is probably throwing off your perception a bit. It's supposed to show the date, unless it's today, then it's the time, or yesterday, as then it says "Yesterday" and the time. Having a much longer string there means there's more to compare everything else to visually.)

And lastly about "sorting current user first" I personally don't like this as to me it seems more confusing than useful but I am fine with waiting for your take on it.

I said "Current user first? (Not sure)" as an idea, not that we should do it, hence the "?" and "(Not sure)"

It should definitely sort by:

  1. user account first
  2. group second (that is: don't ignore the group when sorting, as it could wind up "random")
  3. file/dir name third (as many files and dirs will have the same username and group)

…but that can be pure alphanumeric sorting.

tomasmatus commented 1 month ago

What do you mean by "something special"? Do you have an example?

I don't have any specific ideas. I only saw dolphin shows empty entry and that doesn't fit our design.

I said "Current user first? (Not sure)" as an idea, not that we should do it, hence the "?" and "(Not sure)"

Yes, I know that. I only wanted to share why I chose not to do it.

allisonkarlitskaya commented 1 month ago

Overall LGTM, needs pixel tests and maybe @allisonkarlitskaya wants to do a check as they commented on the PR?

I don't think this really needs pixel tests? This stuff is highly visible in many existing screenshots (the PR updates 20 of them)... Or do you mean for different sorting possibilities?

tomasmatus commented 1 month ago

I think what Jelle meant is that it needed pixeltests update

jelly commented 1 month ago

I think what Jelle meant is that it needed pixeltests update

:100: this :)