Cleanup "Post new comment" form

The "create commit message" button, we need to tackle separately.

But here's a patch to add some licky awesomeness.

This is what I see:

The additional stacking of the (normally two) basic issue data rows, I added at the end... the idea was to decrease the visual height until you see the actual comment area.

Yes, it's quite unusual (and still feels alienate here), so perhaps revert it back to two rows.

ok, back to two rows. Also removed the labels, as you suggested.

I really like this version. However, two issues:

1) Major: At least in Firefox, this form alteration "disables" Firefox's automated form values reset to last known values upon refreshing the page.

I've to study why and under which circumstances this happens. It's possible that the user script runs too early, so Firefox no longer identifies the form to be the same form as before the page refresh...

2) When attaching a file, the form-level submit buttons jump next to the file upload progress bar. I guess the attachments need an additional wrapping DIV container to prevent this.

Thanks for reporting, reviewing, and testing! Committed http://drupal.org/cvs?commit=360112

A new development snapshot will be available within the next 12 hours. This improvement will be available in the next official release.

Isn't this something that should be fixed on d.org itself?

I really don't like how the new version of dreditor removes those labels.

Could you elaborate a bit more on the issues you have with the streamlined comment form? (an annotated screenshot may help)

Aside from that, it's unfortunate that we can't move the issue tags above the comment area via JS. That was one of the optimizations I really liked most. I'm still thinking about very advanced CSS tricks though.

Now that I thought once more about it... it's totally doable :) Committed http://drupal.org/cvs?commit=360476

No annotations, just the result...

Oops, according to #8 this is actually the desired result. Well, then...

humm.... so any belated annotations maybe? :)

This seems to have totally broken the "Create commit message" button. :(

Having used this feature for a few days now, I really wish it was rolled back, or at least there was a way to toggle it off... :\

1. There seems to be a (IMO incorrect) assumption here that only "power users" use Dreditor. But one of the first things I do with new contributors who want to do patch reviews is have them download Dreditor to use the totally awesome patch review tool. And now, they have to expend all kinds of mental energy to figure out what each field is asking about, since they don't have intimate familiarity with this form that most of us do. -1000 for making the issue queue less approachable for newbies, or forcing them to use sub-standard tools so they don't get confused.

2. This totally breaks consistency with every single other form in all of Drupal core and contrib, and the entire Internet as a whole for that matter. Input fields should have labels. They do on Twitter, they do on Facebook, they do on every single other site I can imagine that has a much higher rate of usage than the Drupal.org issue queue. The only form I've ever seen in my life that did not have labels, the contents of the box defaulted to description text that explained to someone new what they were supposed to enter there. These don't. There's not even a little (?) to click on if you get totally stuck.

3. It makes me really nervous relying on a tool, and recommending it to others, that has this kind of stuff just randomly getting shoved into it with basically zero outside review. It calls for either a "modular" system that users can toggle on/off these kinds of "user enhancement" things, or else keeping Dreditor *just* the patch review / commit message tools, and having some "Dreditor Extras" for experimental weird stuff that has nothing to do with the day-to-day reasons we use this tool.

Wow, I really do not like the missing labels on the form elements. :/

> The idea behind this change is to eliminate text one knows anyway, to reduce the amount of reading needed to be done = efficiency increase.

I disagree with that. I don't feel compelled to read all the text I see. I know what all the form elements are, and I skip the labels. But nonetheless their presence adds something. It's hard to explain but it's perhaps something like a white line on the edge of a road: it does nothing for safety whatsoever but somehow gives you a mental guide.

> Sorry but this is contrib,

It is. One module, one purpose. It could be argued that Dreditor is a patch reviewing tool, and adding form tweaks is out of its scope.

> You might not read it, but its there its taking up space and sadly that does put cognitive pressure.

I'm really not an expert on this, but perhaps different people have different cognitive pressures? It sounds like for me and some others, the pressure of not being entirely certain what an item is is slightly greater than having the labels there.

I like how the changelog for this commit was 'Fixed UX of issue comment form.'. Um? That's how we fix UX?

Honestly stuff like this is better fit for a 'power user drupal.org style' like http://userstyles.org/styles/11133 and not dreditor. But that's just my opinion. For everyone else, http://drupalcode.org/viewvc/drupal/contributions/modules/dreditor/dredi... is the link to the code from before this fix was included. Using Greasemonkey you can easily copy that, and then edit the code for your current dreditor and paste that in. If you comment out the last line of the script it will stop warning you about an out-of-date dreditor as well.

I understand we want to innovate. But I think we kinda need to draw a line between functionality that actually makes reviewing and committing patches easier and friendlier, vs 'power user' stuff.

Can we revisit this issue? I actually love the cleaned up issue comment form, but I'm open to do compromises.

If it is only about the hidden labels, then we could change that. That said, the labels are still needless cruft once you know them, so one potential solution would be to move the labels into HTML 'title' attributes, so they show up when hovering over an input element with the mouse. Wouldn't that be a working solution making everyone happy?

Alternatively, we could implement a counter and hide the labels after you've seen them 100 times or so. Yes, there are lots of possibilities. :)

Attached patch implements the title attribute idea. Of course, there's no visual difference compared to the current appearance, and I'm not able to take a screenshot of a browser tooltip, so I'm afraid you have to apply this patch to test it.

Since there are no visual changes, I went ahead and committed attached patch.

Any feedback is welcome.

> so one potential solution would be to move the labels into HTML 'title' attributes, so they show up when hovering over an input element with the mouse

That means if I don't know what things are, I have to spend 5 minutes promenading my mouse round everything and waiting to tooltips to appear on each one. Not very usable.

I still question the entire idea of hiding labels on form fields as a "usability" "feature." Can you cite anywhere at all where this is done elsewhere?

Also generally dismayed that 2 people being +1 to a fundamentally UI changing issue is enough to push it into the tool for everyone. :\

Any ideas for how a toggle could look like? I.e., where could we put that? Perhaps something like the "Create commit message" link/button? A cog in the upper right like contextual links? hrhr...

I don't think people are against a toggle for this, they're just against the fact that this went in *without* a toggle when it has such an impact on a large portion of the user base (the non-experts). I still think this belongs in a user style sheet rather than dreditor since this is only about presentation and not adding features to make life easier. :/ Willing to help review the toggle patch.

I am an expert and still think it's totally bizarre to not have labels on form fields. It's completely non-standard to any form I've ever seen anywhere.

User style++ This would move presentation logic to where it belongs: in the presentation, rather than in the functionality. What Dreditor currently does is akin to hard-coding a call to theme_foo_bar() instead of theme('foo_bar'), and changing the default output of theme_foo_bar() to being totally non-sensical to a huge chunk of its audience.

Sorry for the delay. I've just reverted the hiding of the labels in the issue comment form.

locking comments on this issue due to, well, you know...