Redesigning the sorting toolbar #618

chel-seyy · 2019-03-23T09:44:55Z

Fixes #601

There are many sorting and grouping options for users to choose from, 
e.g. group by author/repo/none, sort by name/repo/contribution.

When grouped by repo, sorting is done within the group. Whereas, when 
grouped by none, sorting is done without any group.

Let's make it clearer for users to choose each sorting option (within
groups or by groups) by having 2 drop-down select menus.

Contribution and variance is not allowed for sorting by repos/author, because additional methods are required to add up the total contribution/variance for each group. Perhaps this can be continued from #486?

Note: Name sorting is performed on usernames, not display names.

fzdy1914

The block is not long enough

fzdy1914

Why do we have a sort within group by repo/branch when group by none and sort group byrepo/branch ?

fzdy1914

This part should have reverse too if you decided to delete reverse checkbox.

chel-seyy · 2019-03-23T14:11:28Z

Why do we have a sort within group by repo/branch when group by none and sort group byrepo/branch ?

To make it clearer, I have disabled sort within group by.

This part should have reverse too if you decided to delete reverse checkbox.

Reversed options are already available here:

eugenepeh

Looks good to me,

Just wondering, is there a way where the option model can directly set the isSortingDesc etc?

codecov · 2019-03-27T02:33:03Z

Codecov Report

Merging #618 into master will increase coverage by 1.44%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master     #618      +/-   ##
============================================
+ Coverage     79.44%   80.89%   +1.44%     
+ Complexity      553      545       -8     
============================================
  Files            71       68       -3     
  Lines          1868     1795      -73     
  Branches        199      190       -9     
============================================
- Hits           1484     1452      -32     
+ Misses          287      249      -38     
+ Partials         97       94       -3

Impacted Files	Coverage Δ	Complexity Δ
src/main/java/reposense/git/GitLog.java	`88.88% <0%> (-5.23%)`	`1% <0%> (-1%)`
src/main/java/reposense/system/CommandRunner.java	`69.23% <0%> (-1.61%)`	`5% <0%> (+2%)`
...osense/git/exception/InvalidFilePathException.java
src/main/java/reposense/util/SystemUtil.java
src/main/java/reposense/git/GitLsTree.java
src/main/java/reposense/report/RepoCloner.java	`63.15% <0%> (+4.42%)`	`10% <0%> (ø)`	⬇️
src/main/java/reposense/git/GitClone.java	`60.86% <0%> (+5.69%)`	`3% <0%> (ø)`	⬇️
src/main/java/reposense/git/GitUtil.java	`97.14% <0%> (+5.76%)`	`13% <0%> (-6%)`	⬇️
...main/java/reposense/authorship/model/FileInfo.java	`70% <0%> (+6.36%)`	`9% <0%> (ø)`	⬇️
src/main/java/reposense/util/StringsUtil.java	`90% <0%> (+20.76%)`	`6% <0%> (ø)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fed3758...bf8d971. Read the comment docs.

frontend/src/static/js/v_summary.js

eugenepeh · 2019-04-04T12:55:50Z

frontend/src/static/js/v_summary.js

      }
    },
+    getOptionWithOrder() {
+      [this.sortingOption, this.isSortingDsc] = this.sortGroupSelection.split(' ');
+      this.isSortingDsc = this.isSortingDsc === 'dsc';


doesn't seems very nice to reassign the variable to become boolean.
Would something like this work?

name and name dsc

if just name, the this.isSortingDsc will automatically be null,
hence can just use if (this.isSortingDsc) to do the logic check.

Yes, that will be much simpler.

…epoSense into toolbar-sorting-601

eugenepeh · 2019-04-05T14:56:39Z

frontend/src/index.jade

+                option(value="totalCommits", v-if="filterGroupSelection === 'groupByNone'") &uarr; contribution
+                option(value="totalCommits dsc", v-if="filterGroupSelection === 'groupByNone'") &darr; contribution
+                option(value="variance", v-if="filterGroupSelection === 'groupByNone'") &uarr; variance
+                option(value="variance dsc", v-if="filterGroupSelection === 'groupByNone'") &darr; variance


just noticed this.

It seems that variance and totalCommits are only enabled when authors wasn't grouped.
Was this the supposed functionality?

Originally intended to continue off from #486, but I can implement it here as well. Is sorting by variance is taken at average?

Originally intended to continue off from #486, but I can implement it here as well. Is sorting by variance is taken at average?

I see. I guess we can continue in #486.

ongspxm

minor nits

frontend/src/static/js/v_summary.js

chel-seyy · 2019-04-09T11:38:33Z

@eugenepeh can this be merged soon, to continue with #486?

eugenepeh · 2019-04-09T15:36:00Z

Nits to the proposed message:

When the ramp charts are grouped together, the sorting is only
performed within the group. This may differs from the expectation of
some users, who may wish to sort them by groups e.g. to list them
accordingly to the group's combined contribution. 

Also, the `reverse` option is not well-presented nor intuitive to the
users enough to understand that it is meant to reverses the sorted
listing. 

Let's fix the mentioned problems by adding another drop-down select
menu to allow the sorting of groups, and include direction arrows in
menu items to replace the need of reverse option.

Note that you should mention the problems more precisely.

Unused code was found in v_summary.js; specifically code associated with the variable, filterSortReverse. They are remnants left behind by the improvement of sort controls introduced in #618. The unused code was originally bind to the reverse checkbox in the older version of RepoSense. As the checkbox was replaced by the introduction of ascending and descending options in #618, the code is now unused and redundant. This is also partially due to the complexity brought upon by new sorting group, the reversing has to be done within the code of descending's logic, rendering those code not reusable. Let's remove the unused code associated to the variable, filterSortReverse and it's implementations.

chel-seyy added 4 commits March 23, 2019 13:29

Refactor sorting select-menu

f8e4284

Implement sorting within groups

20e7031

Enable reverse sorting

0467516

Choose default sorting when group by none

83655bb

chel-seyy added the s.ToReview label Mar 23, 2019

chel-seyy requested a review from a team March 23, 2019 09:44

chel-seyy added 5 commits March 23, 2019 16:48

Remove contribution and variance from sort-groups-by

29f4914

Remove the reverse checkbox

eb6c2ab

Allow sorting via contribution and variance

9e5c433

Return the newline

877de50

Fix codacy

8e6523b

fzdy1914 reviewed Mar 23, 2019

View reviewed changes

fzdy1914 requested changes Mar 23, 2019

View reviewed changes

chel-seyy added 3 commits March 23, 2019 18:15

Add breaklines

c78bd0c

Add breaklines to fix travis

4c41a43

Increase width of label

cbcc6ec

fzdy1914 approved these changes Mar 23, 2019

View reviewed changes

chel-seyy requested a review from a team March 23, 2019 14:30

chel-seyy added 3 commits March 23, 2019 21:52

Remove all choices for sort within group when grouped by none

e0ae775

Redesign dropdown bar

3a1c34c

Place operators at start of line

b8ded91

chel-seyy requested review from a team and removed request for a team March 26, 2019 06:02

eugenepeh reviewed Mar 26, 2019

View reviewed changes

chel-seyy added 2 commits March 27, 2019 10:24

Use computed props to update sorting option and isSortingDsc

7fdd8ee

minor nits

a8f4f6c

chel-seyy requested a review from a team March 27, 2019 06:18

chel-seyy added 2 commits April 1, 2019 13:26

Change to array destructuring

de019f4

Reduce width of labels

62fcc7c

chel-seyy requested a review from a team April 1, 2019 09:37

Add comment

a0923a6

ongspxm reviewed Apr 2, 2019

View reviewed changes

frontend/src/static/js/v_summary.js Outdated Show resolved Hide resolved

chel-seyy added 2 commits April 3, 2019 10:14

Initialize options in data

fd3b5b0

Remove extra coupling

53a1b0e

chel-seyy requested a review from a team April 4, 2019 06:12

Merge branch 'master' into toolbar-sorting-601

bf8d971

yamidark approved these changes Apr 4, 2019

View reviewed changes

chel-seyy requested a review from eugenepeh April 4, 2019 07:29

eugenepeh reviewed Apr 4, 2019

View reviewed changes

chel-seyy requested a review from eugenepeh April 4, 2019 13:41

chel-seyy added 2 commits April 4, 2019 21:25

Remove asc from options

808c7c7

Merge branch 'toolbar-sorting-601' of https://github.com/chelseyong/R…

cc2faf8

…epoSense into toolbar-sorting-601

eugenepeh reviewed Apr 5, 2019

View reviewed changes

eugenepeh approved these changes Apr 5, 2019

View reviewed changes

eugenepeh requested a review from damithc April 5, 2019 16:25

eugenepeh added s.FinalReview and removed s.ToReview labels Apr 5, 2019

eugenepeh requested a review from ongspxm April 5, 2019 16:25

damithc approved these changes Apr 6, 2019

View reviewed changes

ongspxm approved these changes Apr 6, 2019

View reviewed changes

frontend/src/static/js/v_summary.js Show resolved Hide resolved

Add comments

db7066d

eugenepeh merged commit b8033c1 into reposense:master Apr 9, 2019

chel-seyy mentioned this pull request Apr 10, 2019

ChartView: Add remaining features missing from sorting of authors and repos #639

Merged

eugenepeh mentioned this pull request Jun 15, 2019

[#689] v_summary.js: remove unused remnants code #733

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Redesigning the sorting toolbar #618

Redesigning the sorting toolbar #618

chel-seyy commented Mar 23, 2019

fzdy1914 left a comment

fzdy1914 left a comment

fzdy1914 left a comment

chel-seyy commented Mar 23, 2019

eugenepeh left a comment

codecov bot commented Mar 27, 2019 •

edited

eugenepeh Apr 4, 2019

chel-seyy Apr 4, 2019

eugenepeh Apr 5, 2019

chel-seyy Apr 5, 2019

eugenepeh Apr 5, 2019

ongspxm left a comment

chel-seyy commented Apr 9, 2019

eugenepeh commented Apr 9, 2019 •

edited

Redesigning the sorting toolbar #618

Redesigning the sorting toolbar #618

Conversation

chel-seyy commented Mar 23, 2019

fzdy1914 left a comment

Choose a reason for hiding this comment

fzdy1914 left a comment

Choose a reason for hiding this comment

fzdy1914 left a comment

Choose a reason for hiding this comment

chel-seyy commented Mar 23, 2019

eugenepeh left a comment

Choose a reason for hiding this comment

codecov bot commented Mar 27, 2019 • edited

Codecov Report

eugenepeh Apr 4, 2019

Choose a reason for hiding this comment

chel-seyy Apr 4, 2019

Choose a reason for hiding this comment

eugenepeh Apr 5, 2019

Choose a reason for hiding this comment

chel-seyy Apr 5, 2019

Choose a reason for hiding this comment

eugenepeh Apr 5, 2019

Choose a reason for hiding this comment

ongspxm left a comment

Choose a reason for hiding this comment

chel-seyy commented Apr 9, 2019

eugenepeh commented Apr 9, 2019 • edited

codecov bot commented Mar 27, 2019 •

edited

eugenepeh commented Apr 9, 2019 •

edited