Organize labels

[miketaylr]

See discussion starting at #402 (comment)

Also hide status/step/progress category from labels.

[miketaylr]

OK, who volunteers to do this? ^_^

[karlcow]

Maybe we should list the tasks to do:

• Making stats on the current labels.
• Defining categories namespaces.
• Python code to modify to take into account namespaces
• Javascript code to modify to take into account namespaces
• CSS colors to adjust to names (if needed)
• (Then future issue on reorganizing the position of the labels on the page depending on their categories. but this can be a separate issue)

In #402 (comment), we had the proposal

• b-* for browsers such as b-firefox or b-opera
• s-* for status such as s-sitewait or s-needsinfo
• os-* for systems such as os-android or os-ios
• d-* for devices such as d-mobile

[miketaylr]

Python code to modify to take into account namespaces

This should only need to be modified in the issue label bot (to tell it to add whatever prefix/namespace we come up with).

Javascript code to modify to take into account namespaces

Shouldn't need any changes here. The JS doesn't know or care what the labels are, it just passes along whatever is there from/to the server.

[karlcow]

@miketaylr

This should only need to be modified in the issue label bot (to tell it to add whatever prefix/namespace we come up with).

I have a different impression :)
See for example https://github.com/webcompat/webcompat.com/blob/master/webcompat/api/endpoints.py#L106

[miketaylr]

Yes, forgot about that.

[karlcow]

btw somehow, this might makes our life easier to have namespaces in the code. At first I was thinking about moving the category status list to config.py and call that from the different places. But then with the namespaces it could maybe allow us to create a more chameleon code, where the list of valid categories is based on the namespace. There are benefits and pitfalls to both.

The pitfall on basing that on the namespace is that you have to request the label list each time, when it should not be necessary given that it is not changing that often. On the other hand when it is changed it's always up to date.

Intermediate solution the list in a config.py built dynamically when it is deployed.

[miketaylr]

b-* for browsers such as b-firefox or b-opera
s-* for status such as s-sitewait or s-needsinfo
os-* for systems such as os-android or os-ios
d-* for devices such as d-mobile

I wonder if browser-*, status-*, device-* are potentially less confusing (os-* seems fine as is).

[karlcow]

Yes probably a lot better to be explicit. Less confusion. Plus it's something we mostly do not type. And the UI will cater for the namespace anyway.

[karlcow]

ok so far so good. My local instance behaves normally after renaming the labels, aka it fails.

Let's see if I can get the refiltering right.

[karlcow]

After the commits above, I got the right results. 👯

We are not done yet. :)

[karlcow]

First let's fix the test.

→ nosetests
............F...
======================================================================
FAIL: Test that the new filtering is correct.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/karl/code/webcompat.com/tests/test_urls.py", line 106, in test_issues_new
    self.assertEqual(filter_new(issues), result)
AssertionError: '[{"labels": [{"name": "bug"}, {"name": "help wanted"}], "id": 0, "title": "fake bug 0"}, {"labels": [], "id": 1, "title": "fake bug 1"}, {"labels": [{"name": "contactready"}], "id": 2, "title": "fake bug 2"}, {"labels": [{"name": "needsdiagnosis"}], "id": 3, "title": "fake bug 3"}, {"labels": [{"name": "needscontact"}], "id": 4, "title": "fake bug 4"}, {"labels": [{"name": "sitewait"}], "id": 5, "title": "fake bug 5"}]' != '[{"labels": [{"name": "bug"}, {"name": "help wanted"}], "id": 0, "title": "fake bug 0"}, {"labels": [], "id": 1, "title": "fake bug 1"}]'

----------------------------------------------------------------------
Ran 16 tests in 0.703s

FAILED (failures=1)

[karlcow]

Fixed.

→ nosetests
................
----------------------------------------------------------------------
Ran 16 tests in 0.597s

OK

[karlcow]

ok now logically we get UI issues to solve (which is a good thing). The most obvious one being the labels in each bug description. We could just remove the namespace- as a first step. But we have an opportunity to improve a lot more (ping @calexity ) by separating the namespaces (for example browser-* doesn't have to be in the same visible list than status-* etc.)

[karlcow]

I think the functional tests are breaking but it might not be necessary to fix them.
Aka we should change the UI to remove the namespaces from the labels in the DOM.

[miketaylr]

Aka we should change the UI to remove the namespaces from the labels in the DOM.

I can help with that if you need. Would probably be easiest if you pushed your WIP code to a branch on the webcompat repo.

[miketaylr]

But we have an opportunity to improve a lot more

I would recommend just making the changes required to add the namespaces, then iterate on the rest in a separate task.

[karlcow]

I can help with that if you need. Would probably be easiest if you pushed your WIP code to a branch on the webcompat repo.

That would be neat. Guidance might be cool too. Let me do a PR first. so you have the code handy. Note that you need to update your own test repo for adding labels with the right terms. See what I did in my own labels.

TODO: before/if we merge (later on) this PR, stats on the current usage labels and clean up them.

I would recommend just making the changes required to add the namespaces, then iterate on the rest in a separate task.

Agreed. I'm dumping my ideas when they come. My comments are a bit like a notebook.

[karlcow]

Also checking the current milestone. We still have a couple of things to do.

[miketaylr]

OK, merged your WIP code into webcompat issues/436/1 branch.

Note that you need to update your own test repo for adding labels with the right terms.

Documenting for myself, copy the labels from here: https://github.com/karlcow/webcompat-test/labels

And then what's left to close this issue and move on to other issues in the milestone?

• remove namespace from labels in UI
• make sure all tests are passing (see https://travis-ci.org/webcompat/webcompat.com/builds/50647732 for as-is bustage)

Am I forgetting anything?

[hallvors]

(Not sure if this is the right time or place for this request, but I think it would be easier to have separate lists of labels for statuses and browsers. They can still go into the same "labels" field, but if they could be set from different "dropdowns" it would be more usable IMHO)

[karlcow]

@hallvors that's the plan. :)
but first the mechanics. Then the UI.

[miketaylr]

I think the webcompat summit made this escape my TODO list. @karlcow would you like me to proceed with #436 (comment), or would you like to take a stab? Happy either way.

[karlcow]

@miketaylr @magsout In local these two commits don't seem to do anything. They don't break anything, but they don't fix anything. What am I doing wrong?

[miketaylr]

Ah, now I see. @karlcow you're not doing anything wrong you just haven't changed enough. ^_^

The (sub)template (crappy name, yes) that you changed only comes into play when the labels of an issue are re-rendered, after an edit. So if you try your code right now and add a "status-needsinfo" label, for example, it should work as expected after that happens.

In order for the labels to display like you want elsewhere you'll need to do something similar here:

https://github.com/webcompat/webcompat.com/blob/master/webcompat/templates/issue-list.html#L78
https://github.com/webcompat/webcompat.com/blob/master/webcompat/templates/issue.html#L43

That should be it, I think.

One small suggestion, I'd get rid of the newlabel variable and just do the replace directly on label.name:

'<% var newlabel = label.name %>',
'<% newlabel = newlabel.replace(/(browser|status)-/, "") %>',
  '<span class="Label Label--badge" style="background-color:#<%=label.color%>">',
  '<%= newlabel %>',

->

  '<span class="Label Label--badge" style="background-color:#<%=label.color%>">',
  '<%= label.name.replace(/(browser|status)-/, "") %>',

[miketaylr]

❤️

[karlcow]

houla, I had forgotten about this one.
I need to restart the work with a current version of the code.

[karlcow]

When working on a new patch, I realized we have an issue.

status-keyword or browser-keyword will be on github. The way the previous patch is working is that it is removing the status- and browser- part to display as we have currently, but what should happen when we change the tag and send them to github. I guess we need to prefix before sending to github.

Reminder: The objective is to be able to treat separately in the future the status and the browser.

[karlcow]

@miketaylr pointed me to
https://github.com/webcompat/webcompat.com/blob/master/webcompat/static/js/lib/models/issue.js#L85

[miketaylr]

@karlcow are you going to open a pull request?

[miketaylr]

Uh. Nevermind. You did. >_<

[miketaylr]

@karlcow code deployed, label bot updated, and (most) labels renamed.

What do you suggest we do with the following labels:

• windows
• ios
• mozilla
• mobile

I just left them for now. os- probably makes sense for windows and ios. mozilla could be deleted maybe?

[karlcow]

• os-win or os-windows ping @theWebJustWorks/owners ?
• os-ios
• os-fxos
• os-android I have seen for example browser-android instead of os-android in taxiforsure.com - detecting Firefox Android as Desktop browser web-bugs#579
• mobile seems useless in the context of os- I guess it relates more to the discussion in add mobile label miketaylr/web-bugs-issue-tagger#4
• mozilla to delete.