Skip to content

Conversation

@ChristophWurst
Copy link
Contributor

@ChristophWurst ChristophWurst commented Apr 12, 2016

fixes #10788

TODO:

  • save changes to the db
  • write tests

@ChristophWurst ChristophWurst added this to the 9.1-current milestone Apr 12, 2016
@ChristophWurst ChristophWurst changed the title load file sorting mode from the db remember file sort order Apr 12, 2016
Copy link
Contributor

Choose a reason for hiding this comment

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

should the order (asc vs. desc) also be stored? I think yes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yup, I added that in my second commit

@ghost
Copy link

ghost commented Apr 12, 2016

@ChristophWurst

Thanks a lot for your contribution!
Contributions to the core repo require a signed contributors agreement http://owncloud.org/about/contributor-agreement/

Alternatively you can add a comment here where you state that this contribution is MIT licensed.

Some more details about out pull request workflow can be found here: http://owncloud.org/code-reviews-on-github/

@DeepDiver1975
Copy link
Member

@karlitschek can you please add @ChristophWurst to the internal devs? THX

@DeepDiver1975
Copy link
Member

@karlitschek can you please add @ChristophWurst to the internal devs? THX

as long as there are no objections 😉

@ChristophWurst ChristophWurst force-pushed the remember-file-sorting branch 2 times, most recently from 7194b4b to c6bedf5 Compare April 13, 2016 08:38
Copy link
Contributor

Choose a reason for hiding this comment

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

wrong default value

@ChristophWurst ChristophWurst force-pushed the remember-file-sorting branch from c6bedf5 to 4925d04 Compare April 13, 2016 09:40
@ChristophWurst
Copy link
Contributor Author

@nickvergessen thanks for your feedback. Please review again

@ChristophWurst ChristophWurst force-pushed the remember-file-sorting branch from 4925d04 to d00f13d Compare April 13, 2016 10:04
Copy link
Contributor

Choose a reason for hiding this comment

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

Response?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What should it return?

return ['status' => 'success'];

?

Copy link
Contributor

Choose a reason for hiding this comment

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

Well at least not null, I'd suggest to

On success:

return new Response();

On error:

$response = new Response();
$response->setStatus(Http::STATUS_NOT_IMPLEMENTED);
return $response;

and then the doc block should say

@return Response

@karlitschek
Copy link
Contributor

@DeepDiver1975 @ChristophWurst done :-)

@ChristophWurst ChristophWurst force-pushed the remember-file-sorting branch from d00f13d to 576f724 Compare April 13, 2016 12:24
@ChristophWurst
Copy link
Contributor Author

@nickvergessen updated, please check again. thanks.

@jancborchardt
Copy link
Member

Nice, works perfectly! 👍

@nickvergessen
Copy link
Contributor

👍

@DeepDiver1975 DeepDiver1975 merged commit 63a385d into master Apr 15, 2016
@DeepDiver1975 DeepDiver1975 deleted the remember-file-sorting branch April 15, 2016 12:06
@jancborchardt
Copy link
Member

@ChristophWurst is awesome 🚀 :)

@MartinW2
Copy link

Does this allow the admin to define a default sort order for folders as long as they are not 'overwritten' by individual user settings ?
If not, this still doesn't solve the usecase described in 13484. Unfortunately, this has been closed referring to this, which i never understood and agreed with - as reasonable defaults still make sense.
BTW, this is the reason why i don't use owncloud for quite a while now.

@jancborchardt
Copy link
Member

@MartinW2 no, that’s separate. However, sorting by alphabetic order still is the most reasonable default since people are used to it from basically any other file manager. I do agree that we could change to »sort by modified date« at some point for the default. But it definitely does not really make sense for the admin to change that.

Anyway, I guess it might be possible for the admin to force this sorting. @ChristophWurst any idea how @MartinW2 can force that?

@ChristophWurst
Copy link
Contributor Author

One could adjust the default values here, which would then apply only if the user has not yet set any specific sort order.

@jancborchardt
Copy link
Member

@MartinW2 hope that’s sufficient for your use-case.

@MartinW2
Copy link

I already created a hack that worked for me in January 2015 (linked in 13484, pointing here).
Several others stated there that they would like to have the same, and the forum moderator recommended to create a request for this - which i did (13484). But then it was closed.
The point is that it should not be a hack that requires continuous maintenance - i think quite a few admins would like to see that as a standard option.

@jancborchardt
Copy link
Member

What’s the use-case for this option? Admins are not necessarily the best people to dictate UX. Are you rather advocating to change the default to sorting by modified date maybe?

@MartinW2
Copy link

Would you please, finally, have a look at 13484 and it's comments? It's all described there.
Thank you.

@jancborchardt
Copy link
Member

There too, you did not properly answer my question. You would like a change of the default sort order to from alphabetical to modified by date, right?

If you look at our design guidelines, they say among other things:

Software should get out of the way. Do things automatically instead of offering configuration options.

So, instead of simply asking for an option (especially for the admin, which is strange in this case) we should think about if it makes sense to change the default.

@MartinW2
Copy link

Hello Jan-Christoph,

i think the answer is given in a comment for 13484 long time ago.
No, it's not just about changing a default for the sort order. The reason is very clear: For a gallery of pictures, it's reasonable to have the newest pictures on top. For an inventory of text files, sort by filename might be reasonable. So there is no "one fit's all'"default. But in many cases, an admin knows what kind of files are stored in a folder, so he/she can provide a reasonable default.
That's all.
And it's very well described in the comment for 13484.
And it can be easily understood if you would like to understand it.

Sorry Jan-Christoph, but it is this kind of ignorance and "Oh, i am the god of UX, so if i don't see the need, then it will be blocked" attitude, why i was leaving owncloud (apart from some technical flaws).
This is no open source dicussion, it's your kingdom.

As I have moved, I only comment here for the sake of others, who also wait to be able to provide reasonable defaults for their users. You know it can be done easily:

  • provide a general default sort order (no matter what that is)
  • allow (not force) an admin to override this default by an own default
  • allow (not force) users to override the default by individual setting (row header click), per folder

OMG, it would make the UX soooo complicated.
If the god of UX is not willing to support it - simply state it in this way, but don't say "this is covered", as it was done with 13484.

I won't spend further time with this. From my point of view - everything has been said several times now. I am just curious if this comment will be deleted for it's open opinion on how the owncloud "open source" is handled.

Best regards,
Martin

@DeepDiver1975
Copy link
Member

I am just curious if this comment will be deleted for it's open opinion on how the owncloud "open source" is handled.

Just to be clear - we never delete comments (at least I never did and I'm not aware others do) - with the exception of private data or security issues being leaked.

@jancborchardt
Copy link
Member

jancborchardt commented Apr 18, 2016

@MartinW2 well, I read the forum threads and really no one of the others is specifically asking for an admin configuration option. Their use-cases all seem to be covered by the now implemented »remember sort order« functionality.

Apart from that, personal insults and accusations are the opposite of helping. Please read our Code of Conduct, especially the »Be respectful« part. Your comment will definitely not be deleted (as Thomas said), as it mostly says something about your behavior and attitude than it does about our project. I intensively work on ownCloud since some years now and the few times people get personal like this, it really hurts.
As said above, you seem to be the only one specifically asking for an admin setting, so it might as well be you who tries to push your agenda – nevertheless I try to find out what the use-case is. We have a bunch of @owncloud/designers (not just me), and you probably know what happens when an open source project has no designer. Hence statements like »This is no open source dicussion, it's your kingdom.« are plain wrong, and if you were really more involved in the community you would know that.

@skjnldsv
Copy link
Contributor

skjnldsv commented Apr 18, 2016

@MartinW2 it's a shame you decided to talk like that, now I personally find it difficult to continue the dialogue. Because someone disagree with you doesn't mean you're wrong (and the other way around).
However, if you really want your functionality to be implemented, nothing stops you from opening a pull request and fix it your self (like i started doing on owncloud) If there's really many people wanting to add your idea, then we should see it really quickly.

@lock
Copy link

lock bot commented Aug 6, 2019

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Aug 6, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remember changes to sort order

7 participants