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

[WIP] Implement sorting by portion checked within group #538

Closed
wants to merge 1 commit into from

Conversation

pczarn
Copy link
Contributor

@pczarn pczarn commented Jan 1, 2022

How do I hide the button on startup?

Do you think this is a useful feature?

Fixes #444

@pczarn pczarn changed the title [WIP] Implement sorting [WIP] Implement sorting by portion checked Jan 1, 2022
@pczarn pczarn changed the title [WIP] Implement sorting by portion checked [WIP] Implement sorting by portion checked within group Jan 1, 2022
@qarmin
Copy link
Owner

qarmin commented Jan 1, 2022

I'm not sure if I correctly understand what exactly functionality is needed.

In issue 422(not 444) user probably wants to manually sort results by path, name, size etc. column inside groups.
Looks that feature is the first the most wanted one by users, so I don't see reason to not to implement this.
This is already implemented for simple treeviews(without header row) like in Big files or Invalid Symlinks by built-in GTK sort feature(clicking at name of column will sort results).

column.set_sort_column_id(ColumnsBigFiles::Path as i32);

Looking at source code from this PR I don't know what exactly it do.
When I tested it, then sometimes entire group of results jumped to the top of treeview, but usually nothing happens.

@pczarn
Copy link
Contributor Author

pczarn commented Jan 1, 2022

@qarmin The main part of the mechanism is:

list.push((num_selected as f64 / current_index as f64, range));

// ...

    list.sort_by(|&(portion_a, _), &(portion_b, _)| { portion_a.partial_cmp(&portion_b).unwrap() });
    let oldpos: Vec<_> = list.into_iter().flat_map(|(_, range)| range).map(|v| v as u32).collect();
    model.reorder(&oldpos[..]);

When I tested it, then sometimes entire group of results jumped to the top of treeview, but usually nothing happens.

Groups of results with many checked entries will jump to the top or the bottom.

In #444 a user requested a way to go to the next group that has all duplicates selected.

The idea is that if such groups jump to the top of the list, the user can easily go through all such groups.

I can see that a good explanation may be necessary for users to understand this feature. How about a tooltip containing a description, would that be enough?

@qarmin
Copy link
Owner

qarmin commented Jan 2, 2022

Hmmmm....
I always think about button which would move focus to group where all results are selected.
Current solution is quite non-intuitive for me, because moves elements around entire treeview and this will break current sorting in duplicates, where at the top are show the biggest files(hash, size) or with latest name in dictionary(name).

Also sort button is quite misleading, because users probably think that this will sort results by name, path etc.
This is not a problem if to this button will be added options to sort results in group by e.g. path(I think about adding it in future)

@pczarn
Copy link
Contributor Author

pczarn commented Jan 2, 2022

@qarmin ok, you are right about this feature. I will back down on this feature, also because if you didn't grasp it at a first glance, users won't grasp it intuitively as well.

However, it may be useful to repurpose this PR to fix #443 by doing

  • sort by path length within group
    • sort by filename length within group

under the menu introduced in this PR, and adding these plus

  • select all except last
  • select all except first

so that a user may do both actions (sort + select) and arrive at selected only longer path lengths or shorter path lengths, thus fixing issue #443.

For consistency within the sort menu, we may indeed add options to sort results in group by e.g. path, size, like you mentioned earlier:

  • sort by path within group
  • sort by size within group
  • sort by age within group

Is there a need to make a distinction between sorting by filename and by path?

Also I would do a jump menu to cover #292 as well, so that there would be two kinds of jumps

  • jump to the next group with all entries checked
  • jump to the next selected entry

Sounds like a plan?

@pczarn
Copy link
Contributor Author

pczarn commented Jan 20, 2022

@qarmin you're the expert. Could you assign me some issues to work on? Also, since we both speak Polish it would be handy to exchange cell phone numbers (I won't post mine in public, but I can send you mine via reddit). This way, we could discuss whatever needs to be done, on the phone. Is that ok?

@qarmin
Copy link
Owner

qarmin commented Jan 24, 2022

I don't have any strict plans what features/bugfixes should be available in next version, but I think that this issues could be resolved:

but of course any other features can be also merged if not broke too much current app workflow

@micwoj92
Copy link

This branch has conflicts that must be resolved

@pczarn
Copy link
Contributor Author

pczarn commented Sep 27, 2022

We agree that this PR is not the right way forward. Closing.

Thanks for the list of needed features.

@pczarn pczarn closed this Sep 27, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Show next group that has all duplicates selected
3 participants