Fixes #2526 - fixed filteringSort function #2527

magsout · 2018-06-27T15:26:02Z

This PR fixes issue #2526

Proposed PR background

fixed filteringSort function. The order of issue was not stable. Every time we click on filtered button the order of issue changed. I created a global object to save the order and the previous select.

I have read the Pull Request + Code Style Guidelines thoroughly.

TravisBuddy · 2018-06-27T15:29:13Z

Travis tests have failed

Hey @magsout,
Please read the following log in order to understand the failure reason.
It'll be awesome if you fix what's wrong and commit the changes.

1st Build

nosetests

Milestones are not initialized. Check logs.

sleep 5

Writing logs to: /home/travis/build/webcompat/webcompat.com/tmp
Statuses Initialization…
Fetching milestones from Github…
It failed with 403 Client Error: Forbidden for url: https://api.github.com/repos/webcompat/webcompat-tests/milestones!
We will read from data/milestones.json.

Oooops.
We can't find /home/travis/build/webcompat/webcompat.com/data/milestones.json
Double check that everything is configured properly
in config/secrets.py and try again. Good luck!

Milestones are not initialized. Check logs.

$ npm run lint

> webcompat@ lint /home/travis/build/webcompat/webcompat.com
> npm run lint:js && npm run lint:css


> webcompat@ lint:js /home/travis/build/webcompat/webcompat.com
> eslint ./Gruntfile.js ./tests ./grunt-tasks ./webcompat/static/js/lib


> webcompat@ lint:css /home/travis/build/webcompat/webcompat.com
> stylelint './webcompat/static/css/src/**/*.css' './webcompat/static/css/webcompat.dev.css'


$ npm run build

> webcompat@ build /home/travis/build/webcompat/webcompat.com
> grunt

Running "checkDependencies:default" (checkDependencies) task

Running "jst:compile" (jst) task
File webcompat/static/js/dist/templates.js created.

Running "concat:dist" (concat) task

Running "concat:diagnose" (concat) task

Running "concat:issues" (concat) task

Running "concat:issueList" (concat) task

Running "concat:userActivity" (concat) task

Running "uglify:dist" (uglify) task
>> 1 file created 546.15 kB → 385.13 kB

Running "uglify:ga" (uglify) task
>> 1 file created 780 B → 585 B

Running "uglify:issues" (uglify) task
>> 1 file created 48.78 kB → 27.11 kB

Running "uglify:issueList" (uglify) task
>> 1 file created 37.69 kB → 20.14 kB

Running "uglify:userActivity" (uglify) task
>> 1 file created 21.91 kB → 11.36 kB

Running "uglify:diagnose" (uglify) task
>> 1 file created 12.9 kB → 6.44 kB

Running "uglify:contributors" (uglify) task
>> 1 file created 1.1 kB → 737 B

Running "postcss:dist" (postcss) task
>> 1 processed stylesheet created.

Running "cssmin:combine" (cssmin) task
>> 1 file created. 1.8 kB → 36.74 kB

Running "purifycss:target" (purifycss) task
Source Files:  [ 'webcompat/templates/about.html',
  'webcompat/templates/contact.html',
  'webcompat/templates/contributors.html',
  'webcompat/templates/error.html',
  'webcompat/templates/index.html',
  'webcompat/templates/issue.html',
  'webcompat/templates/layout.html',
  'webcompat/templates/list-issue.html',
  'webcompat/templates/new-issue.html',
  'webcompat/templates/privacy.html',
  'webcompat/templates/user-activity.html' ]
Source Files:  [ 'webcompat/templates/about.html',
  'webcompat/templates/contact.html',
  'webcompat/templates/contributors.html',
  'webcompat/templates/contributors/build-tools.html',
  'webcompat/templates/contributors/diagnose-bug.html',
  'webcompat/templates/contributors/organize-webcompat-events.html',
  'webcompat/templates/contributors/report-bug.html',
  'webcompat/templates/contributors/reproduce-bug.html',
  'webcompat/templates/contributors/site-outreach.html',
  'webcompat/templates/contributors/sub-nav.html',
  'webcompat/templates/contributors/web-platform-research.html',
  'webcompat/templates/dashboard/footer.html',
  'webcompat/templates/dashboard/layout.html',
  'webcompat/templates/dashboard/triage.html',
  'webcompat/templates/dashboard/triage/activity-indicator.html',
  'webcompat/templates/dashboard/triage/filters.html',
  'webcompat/templates/dashboard/triage/needstriage-list.html',
  'webcompat/templates/dashboard/triage/no-result.html',
  'webcompat/templates/dashboard/triage/stats.html',
  'webcompat/templates/dashboard/triage/top-bar.html',
  'webcompat/templates/error.html',
  'webcompat/templates/home-page/browse-issues.html',
  'webcompat/templates/home-page/form.html',
  'webcompat/templates/home-page/hero.html',
  'webcompat/templates/home-page/how-it-works.html',
  'webcompat/templates/home-page/join-us.html',
  'webcompat/templates/index.html',
  'webcompat/templates/issue.html',
  'webcompat/templates/issue/issue-aside.html',
  'webcompat/templates/issue/issue-comment-submit.html',
  'webcompat/templates/issue/issue-information.html',
  'webcompat/templates/layout.html',
  'webcompat/templates/list-issue.html',
  'webcompat/templates/list-issue/search-issue-filter.html',
  'webcompat/templates/nav/addon-links.html',
  'webcompat/templates/nav/nav.html',
  'webcompat/templates/nav/shared-nav-links.html',
  'webcompat/templates/nav/topbar.html',
  'webcompat/templates/new-issue.html',
  'webcompat/templates/privacy.html',
  'webcompat/templates/shared/footer.html',
  'webcompat/templates/shared/svg-icons.html',
  'webcompat/templates/user-activity.html',
  'webcompat/templates/user-activity/reported-by-user.html',
  'webcompat/templates/user-activity/user-mentions.html',
  'webcompat/templates/web_modules/pagination.html',
  'webcompat/templates/web_modules/tag.html' ]
Source Files:  [ 'webcompat/templates/issue/aside.jst',
  'webcompat/templates/issue/issue-comment-list.jst',
  'webcompat/templates/issue/issue-labels-sub.jst',
  'webcompat/templates/issue/issue-labels.jst',
  'webcompat/templates/issue/issue-milestones-sub.jst',
  'webcompat/templates/issue/issue-milestones.jst',
  'webcompat/templates/issue/metadata.jst',
  'webcompat/templates/issue/thanks.jst',
  'webcompat/templates/issue/upload-image.jst',
  'webcompat/templates/list-issue/dropdown.jst',
  'webcompat/templates/list-issue/issuelist-issue.jst',
  'webcompat/templates/list-issue/issuelist-search.jst',
  'webcompat/templates/list-issue/issuelist-sorting.jst',
  'webcompat/templates/web_modules/issue-list.jst',
  'webcompat/templates/web_modules/label-editor.jst',
  'webcompat/templates/web_modules/milestone-editor.jst' ]
Source Files:  []
Style Files:  [ 'webcompat/static/css/webcompat.dev.css' ]
Style Files:  [ 'webcompat/static/css/src/animations.css',
  'webcompat/static/css/src/bio.css',
  'webcompat/static/css/src/button.css',
  'webcompat/static/css/src/dropdown.css',
  'webcompat/static/css/src/filter-bar.css',
  'webcompat/static/css/src/filter.css',
  'webcompat/static/css/src/footer.css',
  'webcompat/static/css/src/form.css',
  'webcompat/static/css/src/grid.css',
  'webcompat/static/css/src/hero.css',
  'webcompat/static/css/src/icons.css',
  'webcompat/static/css/src/issue.css',
  'webcompat/static/css/src/issues-list.css',
  'webcompat/static/css/src/label-box.css',
  'webcompat/static/css/src/label-editor.css',
  'webcompat/static/css/src/labels.css',
  'webcompat/static/css/src/nav.css',
  'webcompat/static/css/src/notification-bar.css',
  'webcompat/static/css/src/pagination.css',
  'webcompat/static/css/src/sub-nav.css',
  'webcompat/static/css/src/tiles.css',
  'webcompat/static/css/src/typo.css',
  'webcompat/static/css/src/variables.css' ]

    ________________________________________________
    |
    |   PurifyCSS has reduced the file size by ~ 10.5%  
    |
    ________________________________________________
    

    ________________________________________________
    |
    |   PurifyCSS - Rejected selectors:  
    |   .visually-hidden
    |	.filter-bar-dropdown-listbox ul
    |	.filter-bar-dropdown-listbox a
    |	.filter-bar-dropdown-listbox a:focus
    |	:checked + label .collapsed-text
    |	.filter-bar-dropdown-input:not(:checked) + label .collapsed-text
    |	.filter-bar-dropdown-listbox
    |	:checked ~ .filter-bar-dropdown-listbox
    |	.filter-label .needscontact
    |	.filter-label .needscontact:hover
    |	.filter-label .needscontact.is-active
    |	.filter-label .sitewait
    |	.filter-label .sitewait:hover
    |	.filter-label .sitewait.is-active
    |	.form-upload:hover .visually-hidden
    |	.is-validated .check::after
    |	.is-validated.no-js-error .label-upload
    |	.is-validated.no-js-error:hover .label-upload
    |	.is-validated.no-js-error:focus-within .label-upload
    |	.is-validated .label-remove
    |	.is-validated:hover .icon-remove
    |	.is-validated:focus-within .icon-remove
    |	.is-validated:hover .label-remove .label-icon-message
    |	.is-validated:focus-within .label-remove .label-icon-message
    |	.label-editor-list-item.focused
    |	.label-editorLauncher
    |	.label-editorLauncher.is-active + .label-editor::after
    |	.label-editorLauncher.is-active + .label-editor
    |	.label-editor-launcher:hover
    |	.label-editor-launcher::before
    |	.label-editor-launcher.is-active::before
    |	.wc-Issue-categoryEditor-button .label-editorLauncher
    |	.issue.label-needscontact .x2
    |	.issue.label-sitewait .x2
    |	.label-box-header.label-needscontact
    |	.label-box-editor .label-needscontact
    |	.label-box-header.label-sitewait
    |	.label-box-editor .label-sitewait
    |	.label-box .label-editor-list .label-needscontact
    |	.label-box .label-editor-list .label-sitewait
    |	.is-offscreen
    |	.notification-bar
    |	.notification-information
    |	.notification-alert
    |	.notification-thanks
    |	.is-disabled
    |	.italic
    |	blockquote
    |
    ________________________________________________
    
File "tmp/purestyles.css" created.

Done.

$ nosetests
Milestones are not initialized. Check logs.

npm run test:js -- --reporters="runner" --firefoxBinary=`which firefox`

> webcompat@ test:js /home/travis/build/webcompat/webcompat.com
> node ./tests/functional/_intern.js "--reporters=runner" "--firefoxBinary=/home/travis/firefox-58.0/firefox/firefox"

Error: 
        ======================================================
        Intern checkServer Connection Error: Error: connect ECONNREFUSED 127.0.0.1:5000
        ======================================================
        
  at ClientRequest.<anonymous>  <tests/functional/lib/setup.js:55:9>
  at emitOne  <events.js:116:13>
  at ClientRequest.emit  <events.js:211:7>
  at Socket.socketErrorListener  <_http_client.js:387:9>
  at emitOne  <events.js:116:13>
  at Socket.emit  <events.js:211:7>
  at emitErrorNT  <internal/streams/destroy.js:64:8>
something bad happened inside intern.run()
{ Error: 
        ======================================================
        Intern checkServer Connection Error: Error: connect ECONNREFUSED 127.0.0.1:5000
        ======================================================
        
    at ClientRequest.<anonymous> (/home/travis/build/webcompat/webcompat.com/tests/functional/lib/setup.js:55:9)
    at emitOne (events.js:116:13)
    at ClientRequest.emit (events.js:211:7)
    at Socket.socketErrorListener (_http_client.js:387:9)
    at emitOne (events.js:116:13)
    at Socket.emit (events.js:211:7)
    at emitErrorNT (internal/streams/destroy.js:64:8)
    at _combinedTickCallback (internal/process/next_tick.js:138:11)
    at process._tickCallback (internal/process/next_tick.js:180:9) reported: true }
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! webcompat@ test:js: `node ./tests/functional/_intern.js "--reporters=runner" "--firefoxBinary=/home/travis/firefox-58.0/firefox/firefox"`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the webcompat@ test:js script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /home/travis/.npm/_logs/2018-06-27T15_28_55_292Z-debug.log

laghee · 2018-06-27T16:36:50Z

(Edit: excess of "yet"s)
Disclaimer: I'm not super familiar with all the code yet, and I haven't got tests back up locally to double-check the Travis issue (not sure if those errors are about the new milestone fetching order?), so I'm tagging in a backup reviewer ;p ... r? @miketaylr

So from this it seems like the milestone list of issues (issuesNumber) come through in order of oldest-first, ascending. And since we're initializing on newest, we need to reverse that order, so that seems sensible 👍.

Saving the sort order between filters seems reasonable, too.

I'm curious about the sort option in the dropdown. It doesn't seem to have utility beyond explaining the function, but the user doesn't see it unless they click (which won't have any effect on filtering). Would it make sense, or make any logic simpler/easier if we were to load that as the default option?

miketaylr · 2018-06-27T17:51:19Z

It failed with 403 Client Error: Forbidden for url: https://api.github.com/repos/webcompat/webcompat-tests/milestones!

Hmm... @karlcow I guess a Travis IP will hit the rate limit. I wonder if we need to hard-code milestones.json for tests?

karlcow · 2018-06-27T20:30:41Z

@miketaylr this is #2518

miketaylr · 2018-06-27T20:42:22Z

@miketaylr this is #2518

thanks, it felt familiar.

miketaylr · 2018-06-28T18:21:39Z

I'm curious about the sort option in the dropdown. It doesn't seem to have utility beyond explaining the function, but the user doesn't see it unless they click (which won't have any effect on filtering). Would it make sense, or make any logic simpler/easier if we were to load that as the default option?

@magsout ?

magsout · 2018-06-28T18:45:43Z

@laghee

I'm curious about the sort option in the dropdown. It doesn't seem to have utility beyond explaining the function, but the user doesn't see it unless they click (which won't have any effect on filtering). Would it make sense, or make any logic simpler/easier if we were to load that as the default option?

I'm not sure to understand.

What do you mean by which won't have any effect on filtering and if we were to load that as the default option?

laghee · 2018-06-28T18:59:02Z

Sorry, @magsout, my parentheses sometimes get the better of me!

What do you mean by which won't have any effect on filtering

Selecting sort in the drop down and clicking on filter doesn't change the order of the cards.

and if we were to load that as the default option?

If we loaded the page with Sort selected rather than Newest.
(We could also consider eliminating the sort option altogether and have just Oldest and Newest?)

magsout · 2018-06-28T19:13:06Z

@laghee thanks for the explanations.

(We could also consider eliminating the sort option altogether and have just Oldest and Newest?)

Good idea 👍

magsout · 2018-06-28T19:15:16Z

@laghee @karlcow

webcompat.com/webcompat/views.py

Line 437 in a0dde5e

params = {'per_page': 100, 'sort': 'created', 'direction': 'asc'}

changes the Maybe, changing filter direction by desc would a good idea?

karlcow · 2018-06-29T04:14:53Z

I'm trying to understand. Right now we get a list where the sort is

by creation date
from oldest to newest

The logic at the time it was designed is that the oldest ones are the more urgent.

It's how it is displayed right now.

The way the UI is working right now is that you select the parameters and you have to click "filtered" (which maybe should be "submit") which reorder the full list.

What could be done probably is that maybe it's not necessary to have the "filtered" button and that the reordering is done with onchange

magsout · 2018-06-29T07:01:21Z

@karlcow

The logic at the time it was designed is that the oldest ones are the more urgent.

At the moment, the UI not reflect that.

I need to fix that too. It's a mistake, in this way I make logic simpler in the code.

What could be done probably is that maybe it's not necessary to have the "filtered" button and that the reordering is done with

Yes, we could do that.

magsout · 2018-06-29T07:03:49Z

What I propose

Remove default empty option Sort
made by default oldest in the select (reflect the design)

little bonus? ;)

removed button filter
added an onChange event on every form element.

@karlcow @laghee @softvision-oana-arbuzov @softvision-sergiulogigan , what do you think?

edit: not sure about the button, there is not fetch function, so we can filter the list every time. But it depends how users use the form?

softvision-sergiulogigan · 2018-06-29T07:16:04Z

As far as I can see, the Sort and the Newest are pretty much the same thing (at least in today's list).
Maybe we should default to Oldest

And as for

added an onChange event on every form element.

I's second this.

magsout · 2018-06-29T13:03:39Z

@miketaylr @laghee
updated my commits

laghee

Looks good and works for me locally -- except the filtered button is still there (and throws a type error onClick). Maybe you forgot to remove it?

…stener on elements Form

magsout · 2018-06-29T15:03:52Z

@laghee

Looks good and works for me locally -- except the filtered button is still there (and throws a type error onClick). Maybe you forgot to remove it?

oops, too much cmd+z ;)

fixed ;)

laghee · 2018-06-29T15:58:24Z

🎉

magsout added status: needs review lang: JS scope: dashboard labels Jun 27, 2018

magsout requested a review from laghee June 27, 2018 15:26

laghee requested a review from miketaylr June 27, 2018 16:38

karlcow mentioned this pull request Jun 27, 2018

Testing offline - Milestones initialization #2518

Open

Issue #2526 - fixed filteringSort function and default selected option

212b873

magsout force-pushed the issues/2526 branch from 147b43a to 97d0563 Compare June 29, 2018 13:02

laghee requested changes Jun 29, 2018

View reviewed changes

Issue #2526 - removed <button> Filtered and added an onChange eventLi…

3925efb

…stener on elements Form

magsout force-pushed the issues/2526 branch from 97d0563 to 3925efb Compare June 29, 2018 15:02

laghee approved these changes Jun 29, 2018

View reviewed changes

miketaylr approved these changes Jun 29, 2018

View reviewed changes

miketaylr merged commit d4e9f95 into master Jun 29, 2018

miketaylr added the status: add to changelog label Jun 29, 2018

miketaylr deleted the issues/2526 branch June 29, 2018 16:48

miketaylr removed the status: add to changelog label Jun 29, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes #2526 - fixed filteringSort function #2527

Fixes #2526 - fixed filteringSort function #2527

magsout commented Jun 27, 2018 •

edited

Loading

TravisBuddy commented Jun 27, 2018

laghee commented Jun 27, 2018 •

edited

Loading

miketaylr commented Jun 27, 2018

karlcow commented Jun 27, 2018

miketaylr commented Jun 27, 2018

miketaylr commented Jun 28, 2018

magsout commented Jun 28, 2018

laghee commented Jun 28, 2018

magsout commented Jun 28, 2018

magsout commented Jun 28, 2018

karlcow commented Jun 29, 2018

magsout commented Jun 29, 2018

magsout commented Jun 29, 2018 •

edited

Loading

softvision-sergiulogigan commented Jun 29, 2018

magsout commented Jun 29, 2018

laghee left a comment

magsout commented Jun 29, 2018

laghee commented Jun 29, 2018

Fixes #2526 - fixed filteringSort function #2527

Fixes #2526 - fixed filteringSort function #2527

Conversation

magsout commented Jun 27, 2018 • edited Loading

Proposed PR background

TravisBuddy commented Jun 27, 2018

Travis tests have failed

1st Build

laghee commented Jun 27, 2018 • edited Loading

miketaylr commented Jun 27, 2018

karlcow commented Jun 27, 2018

miketaylr commented Jun 27, 2018

miketaylr commented Jun 28, 2018

magsout commented Jun 28, 2018

laghee commented Jun 28, 2018

magsout commented Jun 28, 2018

magsout commented Jun 28, 2018

karlcow commented Jun 29, 2018

magsout commented Jun 29, 2018

magsout commented Jun 29, 2018 • edited Loading

softvision-sergiulogigan commented Jun 29, 2018

magsout commented Jun 29, 2018

laghee left a comment

Choose a reason for hiding this comment

magsout commented Jun 29, 2018

laghee commented Jun 29, 2018

magsout commented Jun 27, 2018 •

edited

Loading

laghee commented Jun 27, 2018 •

edited

Loading

magsout commented Jun 29, 2018 •

edited

Loading