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

Issue Improvements - Dynamic Information about reporter #630

Merged
merged 3 commits into from
Jun 20, 2015

Conversation

magsout
Copy link
Member

@magsout magsout commented Jun 11, 2015

@miketaylr we need to add dynamic information about reporter

@magsout
Copy link
Member Author

magsout commented Jun 11, 2015

Ouch, I need to rebase

@magsout magsout force-pushed the improvements-issue branch from a2438d3 to 36400c7 Compare June 11, 2015 15:01
@miketaylr
Copy link
Member

^_^

@miketaylr
Copy link
Member

@miketaylr we need to add dynamic information about reporter

Where at? Just the reporters name?

(also these test failures look weird--I'll run them locally at some point today/tomorrow)

@magsout
Copy link
Member Author

magsout commented Jun 11, 2015

Hum need to push some code

@magsout magsout force-pushed the improvements-issue branch 2 times, most recently from 98beb2a to b4c359c Compare June 14, 2015 13:20
@magsout
Copy link
Member Author

magsout commented Jun 14, 2015

Where at? Just the reporters name?

@miketaylr : I add @todo something where you need to add information

@magsout
Copy link
Member Author

magsout commented Jun 14, 2015

Hum, I don't know why travis failed. any idea @miketaylr ?

@magsout magsout force-pushed the improvements-issue branch 3 times, most recently from dfd856f to 2f47e43 Compare June 14, 2015 14:00
@karlcow karlcow changed the title Improvements issue Issue Improvements - Dynamic Information about reporter Jun 15, 2015
@karlcow
Copy link
Member

karlcow commented Jun 15, 2015

I'm not sure I understand the issue and where it's happening. Is it visual or something else?

@magsout
Copy link
Member Author

magsout commented Jun 15, 2015

@karlcow issue page #617

@magsout magsout force-pushed the improvements-issue branch from 2f47e43 to c49512f Compare June 15, 2015 07:35
@magsout magsout force-pushed the improvements-issue branch from c49512f to 0d5e674 Compare June 15, 2015 12:05
@magsout
Copy link
Member Author

magsout commented Jun 15, 2015

So There are only two test failures

× firefox 34.0 on LINUX - issues - issue comments load

AssertionError: Commenter name displayed.: expected '@todo autor' to equal 'miketaylr'

it's ok, when you add @miketaylr the reel author, test will be ok

But for this, I don"t know why.

× firefox 34.0 on LINUX - issues - posting an empty comment fails

AssertionError: Comment was not successfully left.

@miketaylr
Copy link
Member

Oops, missed these emails. Let me take a look @magsout.

@miketaylr
Copy link
Member

I added the info, but still not sure how that's related to the failing tests. Looking at that now.

Also I needed a container to hold the new template (since we rearranged things a bit) so I made a .wc-IssueDetail-report. Feel free to change that to something else @magsout -- just need to change it in the issue.html as well as issues.js.

@miketaylr
Copy link
Member

OK, there we go. There was a .js-issue-comment class that we use to count comment elements. The issue body isn't technically a comment, it just shares some styles so I removed it. That fixed the failures. ^_^

@magsout
Copy link
Member Author

magsout commented Jun 16, 2015

Nice 👍

@magsout
Copy link
Member Author

magsout commented Jun 17, 2015

What do you think @calexity @karlcow @tagawa @hallvors ?

@magsout
Copy link
Member Author

magsout commented Jun 17, 2015

poke @webcompat/owners

@tagawa
Copy link
Member

tagawa commented Jun 17, 2015

Good idea to highlight the reporter details @magsout - a bit of psychological reward.

@magsout
Copy link
Member Author

magsout commented Jun 17, 2015

Quick view:

Before / after

issue

@karlcow
Copy link
Member

karlcow commented Jun 17, 2015

cool. Thanks for the screenshots.
It gives more balance. Suggestions for the Labels
What if, we just remove the label "Labels" and we put it just above the comment box.

The reason is that I often need to go up and down when refreshing the state of the bug, and I see people forgetting about changing the state.

@miketaylr
Copy link
Member

The reason is that I often need to go up and down when refreshing the state of the bug, and I see people forgetting about changing the state.

Another option is to mimic GitHub and have the labels on the right and have them be "sticky". Scroll this PR/bug up and down, for example.

@miketaylr
Copy link
Member

@magsout massive improvements. ^_^

@magsout
Copy link
Member Author

magsout commented Jun 18, 2015

Good idea @miketaylr . I'm concerned about small device and @karlcow's idea is easier for it.

@miketaylr
Copy link
Member

@magsout any reason to not pull this in now?

@magsout
Copy link
Member Author

magsout commented Jun 20, 2015

About label ? Just wainting about what We have to do..

@miketaylr
Copy link
Member

It might be good to ship this (if it's in a merge-able state), and then open a bug for label improvements?

@magsout
Copy link
Member Author

magsout commented Jun 20, 2015

So GO ;)

@miketaylr
Copy link
Member

Sweet.

miketaylr pushed a commit that referenced this pull request Jun 20, 2015
Issue Improvements - Dynamic Information about reporter
@miketaylr miketaylr merged commit 4a954dc into master Jun 20, 2015
@magsout magsout deleted the improvements-issue branch June 20, 2015 16:00
@miketaylr
Copy link
Member

Just filed #637 to follow up with labels. Will try to deploy later today/tonight!

@magsout
Copy link
Member Author

magsout commented Jun 20, 2015

👍

@miketaylr miketaylr restored the improvements-issue branch July 20, 2015 19:04
@miketaylr miketaylr deleted the improvements-issue branch July 21, 2015 04:09
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.

4 participants