Opened 12 years ago

Closed 11 years ago

#2996 closed Enhancement (fixed)

turn error message red

Reported by: dsvensson Owned by: charles
Priority: Normal Milestone: 2.10
Component: Web Client Version: 1.91
Severity: Trivial Keywords:
Cc:

Description

I noticed that some of my torrents were unregistered in the tracker by pure accident. The progress bar is still green. So I whipped up a patch that appends an error class to the progress bar if the this._error is set to > 0, and with additions to common.css the progress bar is now red.

I'm no webmonkey so there might be better places to add this functionality. Input appreciated.

Attachments (4)

red_bar_on_error.diff (1.8 KB) - added by dsvensson 12 years ago.
make progressbar red on errors
make_error_text_red.patch (1.1 KB) - added by fx 11 years ago.
make_error_text_red_via_toggle_class.patch (1.1 KB) - added by fx 11 years ago.
make_error_text_red_play_nice_with_compact_view.patch (1.4 KB) - added by fx 11 years ago.

Download all attachments as: .zip

Change History (21)

Changed 12 years ago by dsvensson

make progressbar red on errors

comment:1 Changed 12 years ago by dsvensson

Updated patch that also fixes the iPhone css file. I don't have IE7 so it would be cool if someone could try that out.

comment:2 Changed 12 years ago by Longinus00

Instead of turning the progress bar red, why not just turn the error text red? The current gtk interface turns all the torrent's text red on an error. If the mac client does something similar I think the web client ought to as well.

comment:3 Changed 12 years ago by charles

I like that idea better because it's a little more consistent with the other GUIs. dsvensson do you want to make a patch for that?

comment:4 Changed 12 years ago by dsvensson

Will do! Thanks for the input!

comment:5 Changed 12 years ago by livings124

  • Summary changed from turn progressbar red on errors to turn error message red

The Mac client does not turn the error text red, but I like the idea for the Web UI.

comment:6 Changed 12 years ago by charles

dsvensson: any progress on this?

comment:7 Changed 12 years ago by charles

  • Keywords feature-disparity added

Changed 11 years ago by fx

comment:8 Changed 11 years ago by fx

  • Keywords feature-disparity removed
  • Version changed from 1.91 to 2.03+

progress

comment:9 Changed 11 years ago by charles

  • Milestone changed from None Set to 2.10
  • Owner changed from kjg to fx
  • Version changed from 2.03+ to 1.91

fx: nice!

Though, when does the error class get removed?

Maybe if it were something like:

var hasError = this.getErrorMessage( ) != undefined;
$(_root._progress_details_container).toggleClass('error',hasError);

What do you think?

Last edited 11 years ago by charles (previous) (diff)

comment:10 Changed 11 years ago by fx

Good point, i didn't think about that. And its a pretty neat way to solve the class issue too. well done.

comment:11 Changed 11 years ago by fx

The latest one works better, takes into account both standard views and compact view

comment:12 Changed 11 years ago by charles

Good point, I didn't think about compact view.

Still, the user might switch between views. Maybe it would be better to remove the "if" and unconditionally edit both object's classes?

comment:13 Changed 11 years ago by fx

The condition is needed because compact view utilises progress details for its peer details text. ie. the text that you see red is peer details in normal view and progress details in compact view. (This is because of the limitation of the positioning of the elements, Im trying to make some time to take a deeper look into the layout of the torrent objects) Appending the class to both objects will turn both peer details and progress details text red (which i tried, and just doesnt look right)

comment:14 Changed 11 years ago by charles

fx: okay. Is make_error_text_red_play_nice_with_compact_view.patch ready to go into trunk?

comment:15 Changed 11 years ago by fx

Yes i beleive it should apply cleanly. Is there anything specific you would like me to change before you apply it?

comment:16 Changed 11 years ago by charles

  • Owner changed from fx to charles
  • Status changed from new to assigned

Nope, just making sure you're happy with it before I pull the trigger.

Added to trunk by r11066

comment:17 Changed 11 years ago by charles

  • Resolution set to fixed
  • Status changed from assigned to closed
Note: See TracTickets for help on using tickets.