Make WordPress Core

Opened 7 months ago

Last modified 8 hours ago

#44839 assigned enhancement

Privacy-data-requests-tables: Remove inline CSS, move it into a css file instead

Reported by: birgire Owned by: desrosj
Milestone: Future Release Priority: low
Severity: normal Version: 4.9.6
Component: Privacy Keywords: has-patch commit
Focuses: Cc:


In WP_Privacy_Data_Export_Requests_Table we have inline CSS:

$download_data_markup .= '<span class="export-personal-data-idle"><button type="button" class="button-link export-personal-data-handle">' . __( 'Download Personal Data' ) . '</button></span>' .
  '<span style="display:none" class="export-personal-data-processing" >' . __( 'Downloading Data...' ) . '</span>' .
  '<span style="display:none" class="export-personal-data-success"><button type="button" class="button-link export-personal-data-handle">' . __( 'Download Personal Data Again' ) . '</button></span>' .
  '<span style="display:none" class="export-personal-data-failed">' . __( 'Download has failed.' ) . ' <button type="button" class="button-link">' . __( 'Retry' ) . '</button></span>';

  <span class="export-personal-data-idle"><button type="button" class="button export-personal-data-handle"><?php _e( 'Email Data' ); ?></button></span>
  <span style="display:none" class="export-personal-data-processing button updating-message" ><?php _e( 'Sending Email...' ); ?></span>
  <span style="display:none" class="export-personal-data-success success-message" ><?php _e( 'Email sent.' ); ?></span>
  <span style="display:none" class="export-personal-data-failed"><?php _e( 'Email could not be sent.' ); ?> <button type="button" class="button export-personal-data-handle"><?php _e( 'Retry' ); ?></button></span>

and in WP_Privacy_Data_Removal_Requests_Table we have:

  $remove_data_markup .= '<span class="remove-personal-data-idle"><button type="button" class="button-link remove-personal-data-handle">' . __( 'Force Erase Personal Data' ) . '</button></span>' .
    '<span style="display:none" class="remove-personal-data-processing" >' . __( 'Erasing Data...' ) . '</span>' .
    '<span style="display:none" class="remove-personal-data-failed">' . __( 'Force Erase has failed.' ) . ' <button type="button" class="button-link remove-personal-data-handle">' . __( 'Retry' ) . '</button></span>';

    <span class="remove-personal-data-idle"><button type="button" class="button remove-personal-data-handle"><?php _e( 'Erase Personal Data' ); ?></button></span>
    <span style="display:none" class="remove-personal-data-processing button updating-message" ><?php _e( 'Erasing Data...' ); ?></span>
    <span style="display:none" class="remove-personal-data-failed"><?php _e( 'Erasing Data has failed.' ); ?> <button type="button" class="button remove-personal-data-handle"><?php _e( 'Retry' ); ?></button></span>

I wonder if we could remove this inline CSS and instead add something like:

.export-personal-data-failed {
	display: none;

in list-tables.css ?

Attachments (2)

44839.diff (5.0 KB) - added by pratikthink 7 months ago.
As birgire suggested
44839.2.diff (5.2 KB) - added by xkon 15 hours ago.
removes inline css, adds hidden class, updates xfn.js

Download all attachments as: .zip

Change History (13)

#1 @SergeyBiryukov
7 months ago

  • Keywords needs-patch added

7 months ago

As birgire suggested

#2 @pratikthink
7 months ago

  • Keywords has-patch needs-testing added; needs-patch removed

#3 @garrett-eclipse
5 months ago

  • Milestone changed from Awaiting Review to 5.0.1

#4 @pento
3 months ago

  • Milestone changed from 5.0.1 to 5.0.2

#5 @pento
3 months ago

  • Milestone changed from 5.0.2 to 5.0.3

#6 @desrosj
3 months ago

  • Milestone changed from 5.0.3 to 5.1

This needs testing still. Let's punt to 5.1.

#7 @desrosj
3 months ago

  • Owner set to desrosj
  • Status changed from new to assigned

Thanks for the patch, @pratikthink!

@birgire my initial testing looks good for this. Are you able to give this some testing?

I also think we should utilize the .hidden class that already exists for this instead.

#8 @pento
3 months ago

  • Keywords needs-refresh added
  • Milestone changed from 5.1 to Future Release

I'm inclined to agree that it should use the .hidden class.

This should probably also update setActionState() in xfn.js to add/remove the hidden class, instead of using hide() and show().

15 hours ago

removes inline css, adds hidden class, updates xfn.js

#9 @xkon
15 hours ago

  • Keywords needs-refresh removed

44839.2.diff removes the inline css that we've added initially and instead uses the .hidden class.

This ticket was mentioned in Slack in #core-privacy by xkon. View the logs.

15 hours ago

#11 @garrett-eclipse
8 hours ago

  • Keywords commit added; needs-testing removed

Thanks @xkon testing this it works nicely using the .hidden over the inline css and hide/show. I tested on both Export and Erase screens. I'm marking it for commit, but am unsure if it missed the 5.2 ship so am leaving on the Future Milestone.

Note: See TracTickets for help on using tickets.