WordPress.org

Make WordPress Core

Opened 6 years ago

Closed 6 years ago

#5369 closed enhancement (fixed)

jQuery should be used to fade alerts

Reported by: JeremyVisser Owned by: mdawaffe
Milestone: 2.5 Priority: low
Severity: minor Version: 2.5
Component: Administration Keywords:
Focuses: Cc:

Description

jQuery has the ability to perform fades now, I believe. Doing fades with jQuery should simplify code, and would also give us the ability to drop fat.js.

Additionally, FAT will only fade an alert if it contains an id="something", not just class="fade alert", which is a pain in the butt for debugging, IMO. jQuery doesn't suffer from that limitation.

Attachments (4)

jquery-alerts.diff (7.5 KB) - added by JeremyVisser 6 years ago.
jquery-alerts.2.diff (8.3 KB) - added by JeremyVisser 6 years ago.
Uses the jquery-color plugin. Non-working, because it requires jQuery 1.2, and WP only has 1.1.4.
jquery-alerts.3.diff (4.5 KB) - added by JeremyVisser 6 years ago.
Here's another one, using the Highlight function of interface.js.
fade.diff (5.6 KB) - added by ryan 6 years ago.
Fade with jquery.color

Download all attachments as: .zip

Change History (32)

comment:1 lloydbudd6 years ago

  • Milestone changed from 2.4 to 2.5

comment:3 JeremyVisser6 years ago

  • Owner changed from anonymous to JeremyVisser
  • Status changed from new to assigned

[citation needed]

I can't for the life of me figure out how I came up with that "jQuery has the ability to perform fades now" statement. jQuery has no such colour fading support.

However, I found a jQuery-ified version of FAT, which I have baked into a patch, which will give plugin authors the ability to drop the id="mymessage" attribute from their alerts (by just using class="fade alert".

It has the wonderful side-effect of pointlessly adding jQuery as a dependency for every WP Admin page now, so no WP Admin page won't have jQuery enqueued.

JeremyVisser6 years ago

comment:4 JeremyVisser6 years ago

A side-effect for this patch: Fat.fade_all() is no longer called unless there is a .alert.fade present, so if you manually create a <div id="mymessage" class="fade-ff0000 alert"> (i.e. a red fading alert), it won't fade anymore. You could always call jQuery(".alert#mymessage").vkfade("ff0000") but that's inferior to just using FAT.

Also, I strongly believe the "fade" class is unsemantic, and the "alert" class for alerts should be just used, and the fade should be implied. For red fades, an "error" class could be added.

Could FAT be rewritten to use jQuery.animate()?

Ironically, I'm currently -1 to my own patch in its current state.

comment:5 follow-up: filosofo6 years ago

I think you could use instead the Highlight effect of the Interface plugin, a jQuery plugin which is included in WP (and used on the admin widgets page).

comment:6 JeremyVisser6 years ago

Ah, found what I was looking for, with a usage example on the release notes.

However, I was getting really weird JS errors, and realised that this requires jQuery 1.2. Are we ready to make the move to 1.2 yet?

JeremyVisser6 years ago

Uses the jquery-color plugin. Non-working, because it requires jQuery 1.2, and WP only has 1.1.4.

comment:7 in reply to: ↑ 5 JeremyVisser6 years ago

Replying to filosofo:

I think you could use instead the Highlight effect of the Interface plugin, a jQuery plugin which is included in WP (and used on the admin widgets page).

Cool, I'll have a hack at that.

JeremyVisser6 years ago

Here's another one, using the Highlight function of interface.js.

comment:8 follow-up: JeremyVisser6 years ago

I'm a bit uneasy about the need for a 4-line .js file. Should this be integrated anywhere else, like at the end of interface.js?

comment:9 in reply to: ↑ 8 DD326 years ago

Replying to JeremyVisser:

I'm a bit uneasy about the need for a 4-line .js file. Should this be integrated anywhere else, like at the end of interface.js?

Interface is a jQuery plugin, so in my mind it should stay as *only* interface.

Maybe a Generic JS file for around the admin pages is needed? (Or someone could revisit the JS / CSS combiner+compressor function idea again)

comment:10 follow-up: matt6 years ago

interface.js is HUGE.

comment:11 in reply to: ↑ 10 rob1n6 years ago

Replying to matt:

interface.js is HUGE.

No kidding. It also seems to be unmaintained currently -- I believe that's why the official jQuery UI project was started.

comment:12 ryan6 years ago

Once we upgrade to jQuery 1.2 (see #549), can we use the color animations jquery plugin?

http://plugins.jquery.com/project/color

comment:13 ryan6 years ago

Duh, I see that there's already a patch for that. That route seems the best since we're planning to upgrade jquery.

ryan6 years ago

Fade with jquery.color

comment:14 ryan6 years ago

Patch uses jquery color animations to fade. We still need fat until list manipulation is ported to jquery color.

comment:15 mdawaffe6 years ago

jQuery.color is a bit nicer than FAT. It's also 1k bigger, though.

Maybe WP should start minifying some of its JS?

comment:16 ryan6 years ago

  • Resolution set to fixed
  • Status changed from assigned to closed

(In [6572]) Fade using jquery.color. Props JeremyVisser. fixes #5369

comment:17 ryan6 years ago

  • Milestone changed from 2.6 to 2.5

Minified jquery.color. Created admin.js to hold the fade code and any future JS bits that should be loaded for all admin pages.

comment:18 ryan6 years ago

(In [6573]) Fade using jquery.color. Props JeremyVisser. fixes #5369

comment:19 follow-up: matt6 years ago

Did we just lose the ability in FAT to go to/from any background color? (Akismet uses this.)

Fat was smaller and more functional, and didn't use Prototype. Switching for switching sake is not worth it.

comment:20 matt6 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

comment:21 in reply to: ↑ 19 ryan6 years ago

Replying to matt:

Did we just lose the ability in FAT to go to/from any background color? (Akismet uses this.)

I don't think so, but need to make sure.

Fat was smaller and more functional, and didn't use Prototype. Switching for switching sake is not worth it.

If we use core animations to bring back the fading delete when deleting from lists then it is probably worth it. But, 'tis no matter to me to revert.

comment:22 ryan6 years ago

Do you mean the fade-ff0000? jquery.color doesn't seem to have such a convenience. Easily written, but recreating fat is not worthwhile.

comment:23 ryan6 years ago

(In [6574]) Revert back to fat for now. see #5369

comment:24 ryan6 years ago

Back to FAT pending further revelations, if any.

comment:25 matt6 years ago

Actually I think we can kill the fade-trick. I think we can make our fading more static and leave fancier stuff up to something else. Let me look at a cleaner way of loading fat/color though, so we don't get the hit on every page even if there's nothing to fade.

comment:26 matt6 years ago

(In [6582]) Replacing FAT with jQuery effects, new styling for alerts. See #5369.

comment:27 matt6 years ago

  • Owner changed from JeremyVisser to mdawaffe
  • Status changed from reopened to new

Aw crap, Trac just deleted my long comment. Short version:

jQuery.color is generally useful to WP, allows animating non numeric color fields. Means we don't need effects libraries for things like the pulse on the new notices.

Leaving ticket open until mdawaffe has a chance to review wp-lists.js, which I modified to the best of my ability, but I feel I missed something. Also listmanip stuff should be removed from the repo.

mdawaffe, there is something funky going on line 65. When I have 'break on all errors" in firebug it catches every time I delete a comment, and the infinite loading feature seems to break after one. I see the AJAX request returning a new comment but there's nothing added to the list.

comment:28 mdawaffe6 years ago

  • Resolution set to fixed
  • Status changed from new to closed
  • Version set to 2.5

matt, fades seem to work now [7050].

Deletes work fine for me.

Infinite comments does not work for me. Opening new ticket.

Note: See TracTickets for help on using tickets.