search

thomaslevesque / CosmosDBStudio

A tool to browse and query Azure Cosmos DB databases
MIT License
54 stars 12 forks source link

Sort tree nodes alphabetically #56

Closed ricardopinedathen closed 2 years ago

ricardopinedathen commented 2 years ago

See https://github.com/thomaslevesque/CosmosDBStudio/issues/50

thomaslevesque commented 2 years ago

Hi @ricardopinedathen, Why did you close this PR? I merged the other one, but haven't tested this one yet...

ricardopinedathen commented 2 years ago

Hi @ricardopinedathen, Why did you close this PR? I merged the other one, but haven't tested this one yet...

@thomaslevesque Sorry about that, closed by mistake 🤦‍♂️!

ricardopinedathen commented 2 years ago

@thomaslevesque Changes implemented, let me know if you're ok with the naming I used, otherwise I can change them as you like. Tested with these items: image

thomaslevesque commented 2 years ago

@ricardopinedathen actually, I think there's a simpler and cleaner way to do it. Re-reading my comment made me think:

the DisplayName is really a UI concern, but the classes in this namespace represent actual Cosmos DB resources, so I'd rather not expose the DisplayName directly on them. Ideally it wouldn't even be in these classes, but I realize it's more convenient this way.

The ITreeNode interface is really purely a UI concern, so it doesn't make sense for the model classes to implement it. However, Model classes are always wrapped in ViewModel classes that derives from TreeNodeViewModel, with a Text property... So the abstraction you introduced with ITreeNode already exists in the ViewModel layer. I think the best approache is simply to apply the sort after the conversion to a viewmodel.

This can even be refactored in a single place (except for the root nodes): in NonLeafTreeNodeViewModel.InternalLoadChildrenAsync, you can just apply the sort like this:

_children = (await Task.Run(LoadChildrenAsync)).OrderBy(n => n.Text);
ricardopinedathen commented 2 years ago

@ricardopinedathen actually, I think there's a simpler and cleaner way to do it. Re-reading my comment made me think:

the DisplayName is really a UI concern, but the classes in this namespace represent actual Cosmos DB resources, so I'd rather not expose the DisplayName directly on them. Ideally it wouldn't even be in these classes, but I realize it's more convenient this way.

The ITreeNode interface is really purely a UI concern, so it doesn't make sense for the model classes to implement it. However, Model classes are always wrapped in ViewModel classes that derives from TreeNodeViewModel, with a Text property... So the abstraction you introduced with ITreeNode already exists in the ViewModel layer. I think the best approache is simply to apply the sort after the conversion to a viewmodel.

This can even be refactored in a single place (except for the root nodes): in NonLeafTreeNodeViewModel.InternalLoadChildrenAsync, you can just apply the sort like this:

_children = (await Task.Run(LoadChildrenAsync)).OrderBy(n => n.Text);

Well, that's definitely easier, guess this was more of a MVVM refresher for me! I'll close this PR and submit a new one with only these changes, as they're basically two lines.