Make WordPress Core

Opened 9 years ago

Closed 9 years ago

Last modified 9 years ago

#30885 closed defect (bug) (fixed)

Move alert boxes with class notice

Reported by: sippis's profile sippis Owned by: azaozz's profile azaozz
Milestone: 4.1.1 Priority: low
Severity: minor Version: 4.1
Component: Administration Keywords: has-patch commit fixed-major
Focuses: ui, javascript Cc:

Description

In #27418 there were added new main class for admin notices, but javascript wasn't updated to work with it.

Right now javascript moves divs "updated" and "error" to the wrap div. This should also be done for divs with class "notice".

Current wp-admin/js/common.js:371-373

// Move .updated and .error alert boxes. Don't move boxes designed to be inline.
$('div.wrap h2:first').nextAll('div.updated, div.error').addClass('below-h2');
$('div.updated, div.error').not('.below-h2, .inline').insertAfter( $('div.wrap h2:first') );

Updating those lines to following fixes this problem

// Move .updated, .error and .notice alert boxes. Don't move boxes designed to be inline.
$('div.wrap h2:first').nextAll('div.updated, div.error, div.notice').addClass('below-h2');
$('div.updated, div.error, div.notice').not('.below-h2, .inline').insertAfter( $('div.wrap h2:first') );

Attachments (1)

Screenshot 2015-01-02 10.54.37.png (21.3 KB) - added by sippis 9 years ago.

Download all attachments as: .zip

Change History (10)

#1 @sippis
9 years ago

  • Summary changed from Move new alert boxes with class notice to Move alert boxes with class notice

#2 @azaozz
9 years ago

  • Milestone changed from Awaiting Review to 4.2
  • Severity changed from normal to minor

Thanks @sippis, looks good. Can also reuse one of the selectors and fix the syntax a bit.

#3 @azaozz
9 years ago

  • Owner set to azaozz
  • Resolution set to fixed
  • Status changed from new to closed

In 31023:

Move the (recently added) .notice admin notices below the first H2, same as the .updated and .error notices. Props sippis, fixes #30885.

This ticket was mentioned in Slack in #core by sippis. View the logs.


9 years ago

#5 @ocean90
9 years ago

  • Keywords has-patch commit fixed-major added
  • Milestone changed from 4.2 to 4.1.1
  • Priority changed from normal to low
  • Resolution fixed deleted
  • Status changed from closed to reopened

Moving for 4.1.1 consideration.

#6 follow-up: @johnbillion
9 years ago

I think we should pull this JS out altogether and leave admin notices at the top of the page. It causes admin notices to jump around, especially if the browser or the page is slow to load.

Thoughts?

This ticket was mentioned in Slack in #core by dd32. View the logs.


9 years ago

#8 @dd32
9 years ago

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

In 31434:

Move the (recently added) .notice admin notices below the first H2, same as the .updated and .error notices.

Props sippis.
Merges [31023] to the 4.1 branch.
Fixes #30885.

#9 in reply to: ↑ 6 @dd32
9 years ago

Replying to johnbillion:

I think we should pull this JS out altogether and leave admin notices at the top of the page. It causes admin notices to jump around, especially if the browser or the page is slow to load.

Thoughts?

I think we all hate it (slack), and the notices should be output in the correct place in the first place, sounds like something worth looking at in a new ticket

Note: See TracTickets for help on using tickets.