WordPress.org

Make WordPress Core

Opened 16 months ago

Last modified 41 hours ago

#44135 accepted enhancement

Have Erasure button workflow follow Export button workflow replacing with static link

Reported by: garrett-eclipse Owned by: garrett-eclipse
Milestone: 5.3 Priority: normal
Severity: normal Version: 4.9.6
Component: Privacy Keywords: has-patch commit
Focuses: administration Cc:

Description

Hello,

On the export workflow when you click the Email Data button it ends up going away replaced with a static ‘Email Sent’ message
On the removal workflow when you click the remove you get the inline alert but the buttons goes back to actionable state which makes you feel you should click it again. should it not instead go to a static text of ‘Data Removed’ to avoid repeatedly clicking.

Thanks

Attachments (14)

Screen Shot 2018-05-17 at 2.02.45 PM.png (16.1 KB) - added by garrett-eclipse 16 months ago.
Email Sent
Screen Shot 2018-05-17 at 2.02.39 PM.png (23.2 KB) - added by garrett-eclipse 16 months ago.
Data Erased
44135.diff (2.1 KB) - added by allendav 16 months ago.
erase-completed-1.jpg (30.0 KB) - added by birgire 16 months ago.
erase-completed-2.jpg (30.9 KB) - added by birgire 16 months ago.
44135.2.diff (2.5 KB) - added by allendav 16 months ago.
Dark non clickable privacy request row actions slightly
erase-completed-1b.jpg (20.7 KB) - added by birgire 16 months ago.
Screen Shot on 2018-07-09 at 07_35_24.png (158.6 KB) - added by birgire 15 months ago.
44135.3.diff (2.6 KB) - added by birgire 14 months ago.
44135.4.diff (2.5 KB) - added by garrett-eclipse 4 months ago.
Refreshed patch
Screen Shot 2019-06-06 at 12.15.44 AM.png (74.4 KB) - added by garrett-eclipse 4 months ago.
Updated row-actions to show better contrast
44135.5.diff (3.0 KB) - added by garrett-eclipse 41 hours ago.
Updated patch to correct verbiage thanks @pputzer
3ae775ee1e9b816104bb2e087a413f81.gif (185.4 KB) - added by garrett-eclipse 41 hours ago.
Standard Erasure showing 'Erasure completed'
433c46b14afdb07dccbd892d74fa9c71.gif (182.5 KB) - added by garrett-eclipse 41 hours ago.
Force Erasure showing 'Erasure completed'

Download all attachments as: .zip

Change History (51)

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


16 months ago

#2 @desrosj
16 months ago

  • Keywords needs-patch ui-feedback added
  • Milestone changed from Awaiting Review to 4.9.7
  • Version changed from trunk to 4.9.6

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


16 months ago

@allendav
16 months ago

#4 @allendav
16 months ago

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

To test:

  • Create a new request
  • Use the Force Erase Personal Data action flow to erase their data
  • Ensure the action text changes to "Erase completed." when it completes.
  • Resend the request to get it back to pending state
  • Confirm the request email
  • Use the Erase Personal Data button to erase their data
  • Ensure the button is replaced with "Erase completed." when it completes

I chose "Erase completed." instead of "Data removed" in the event no data was found to erase - that way the message still makes sense.

Last edited 16 months ago by allendav (previous) (diff)

#5 @birgire
16 months ago

Thanks @allendav for the patch and detailed test steps.

I tested the patch and followed the test steps:

✔️ Create a new request

✔️ Use the Force Erase Personal Data action flow to erase their data

✔️ Ensure the action text changes to "Erase completed." when it completes.

See erase-completed-1.jpg

I noticed that the action row text is hardly readable because of the light coloring:

.row-actions {
    color: #ddd;
    ...
}

Could we make it darker? E.g. with color: #555;, same color as .widefat td and .widefat th.

The same goes for other row action texts, e.g. the Download has failed. for erase requests.

✔️ Resend the request to get it back to pending state

✔️ Confirm the request email

✔️ Use the Erase Personal Data button to erase their data

✔️ Ensure the button is replaced with "Erase completed." when it completes

See erase-completed-2.jpg

I chose "Erase completed." instead of "Data removed" in the event no data was found to erase - that way the message still makes sense.

That makes sense.

Last edited 16 months ago by birgire (previous) (diff)

@allendav
16 months ago

Dark non clickable privacy request row actions slightly

#6 @allendav
16 months ago

@birgire - in 44135.2.diff I changed the color to 999 - let me know what you think - I wanted to still convey that the hover cell action was disabled/not-clickable, so I didn't go for the full on 555 like in other contexts for these tables.

#7 follow-up: @birgire
16 months ago

I wanted to still convey that the hover cell action was disabled/not-clickable, so I didn't go for the full on 555 like in other contexts for these tables.

erase-completed-1b.jpg shows the #999 color.

The #999 color looks suitable for a disabled context and it's also still easier to read than #ddd. So this looks good to me.

(I haven't checked though, if there's a standardized admin color for this context)

@allendav, when trying to test the css, I see why you created ticket #44328 :-)

Version 0, edited 16 months ago by birgire (next)

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


15 months ago

#9 @ocean90
15 months ago

  • Milestone changed from 4.9.7 to 4.9.8

4.9.7 has been released, moving to next milestone.

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


15 months ago

#11 in reply to: ↑ 7 @JoshuaWold
15 months ago

Replying to birgire:

I wanted to still convey that the hover cell action was disabled/not-clickable, so I didn't go for the full on 555 like in other contexts for these tables.

erase-completed-1b.jpg shows the #999 color.

The #999 color looks suitable for a disabled context and it's also still easier to read than #ddd. So this looks good to me.

(I haven't checked though, if there's a documented standardized admin color for this context)

@allendav, when trying to test the css, I see why you created ticket #44328 :-)

Howdy! Responding per our bug-scrub today in #Core. I checked the #999 color against two contrast checkers; just to see if it passes accessibility needs (https://webaim.org/resources/contrastchecker/ and https://contrastchecker.com/).

#999 fails all the accessibility checks. # 72777C seems to pass all but one contrast test, and we're already using it in the admin. See here: https://d.pr/i/3LTFzm.

If someone from the accessibility team can weigh in that'd be awesome! Otherwise my recommendation would be to go with # 72777C

Last edited 15 months ago by JoshuaWold (previous) (diff)

This ticket was mentioned in Slack in #design by joshuawold. View the logs.


15 months ago

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


15 months ago

#14 @desrosj
15 months ago

  • Keywords needs-refresh added; ui-feedback removed

Needs color adjusted per @JoshuaWold's feedback.

#15 @birgire
15 months ago

@JoshuaWold thanks for checking it out and the links. I attached your screenshot here Screen Shot on 2018-07-09 at 07_35_24.png just in case the link goes down or changes.

#16 @karmatosed
15 months ago

I would note a caution in double colouring as seen with blue and green combined in messages. I have to ask why are we showing blue and green together? I would say the following works:

  • No color around the actual email itself.
  • If successful green
  • If no personal data found blue

Grey doesn't really mean anything in terms of this UI so I think lets just not have a double notice color combination and we're set. It after all should only have a notice for information about the state change, not wrap everything.

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


15 months ago

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


14 months ago

@birgire
14 months ago

#19 @birgire
14 months ago

@karmatosed thanks, I wonder if it would be better served as a new ticket, regarding e.g. the mixing of blue/green notices.

44135.3.diff updates the color from #999 to the better contrast of #72777C, as suggested by @JoshuaWold.

#20 @pbiron
14 months ago

  • Keywords commit added; needs-refresh removed

#21 @pbiron
14 months ago

  • Keywords commit removed

#22 @pbiron
14 months ago

It was just decided that we will do 2 betas for 4.9.8. So, I removed the commit keyword and this ticket will not be included in beta 1, giving us one more week to hash out the details of the colors for beta 2.

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


14 months ago

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


14 months ago

#25 @desrosj
14 months ago

  • Milestone changed from 4.9.8 to 4.9.9

4.9.8 RC is out. Punting to 4.9.9.

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


14 months ago

#27 @pento
12 months ago

  • Milestone changed from 4.9.9 to Future Release

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


12 months ago

#29 @garrett-eclipse
8 months ago

  • Keywords needs-refresh ui-feedback ux-feedback added

It seems this needs some final decisions on colours and then a final refresh to move back into testing and commit.

#30 @garrett-eclipse
6 months ago

Related - #44133
The indication of no personal data in export as a notice in admin was raised while working on the above.

#31 @garrett-eclipse
4 months ago

  • Keywords needs-refresh ui-feedback ux-feedback removed
  • Owner set to garrett-eclipse
  • Status changed from new to accepted

Thanks @allendav / @birgire for the starting patches.

I've refreshed them accounting for two items;

  • Updated to account for the privacy re-org from #43895
  • Replaced 'display:none;' usage with .hidden as that path was paved with #44839

As well as branched @karmatosed feedback (thank you) into a separate ticket so we don't lose Trac of it.
#47491

Also greatly appreciate the initial design feedback @JoshuaWold.

Please test and we'll line up for 5.3.

@garrett-eclipse
4 months ago

Refreshed patch

@garrett-eclipse
4 months ago

Updated row-actions to show better contrast

#32 @garrett-eclipse
4 months ago

  • Milestone changed from Future Release to 5.3

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


2 months ago

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


4 days ago

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


2 days ago

#36 @pputzer
2 days ago

@garrett-eclipse Is "Erase completed" correct English? (To me it seems it should be either "Erase operation completed" or "Erasure completed", since "erase" is a verb.

@garrett-eclipse
41 hours ago

Updated patch to correct verbiage thanks @pputzer

@garrett-eclipse
41 hours ago

Standard Erasure showing 'Erasure completed'

@garrett-eclipse
41 hours ago

Force Erasure showing 'Erasure completed'

#37 @garrett-eclipse
41 hours ago

  • Keywords commit added; needs-testing removed

Good catch thanks @pputzer, in 44135.5.diff I've updated incorrect strings to the following;

  • Erasure completed.
  • Data Erasure has failed.
  • Force Erasure has failed.

As well as ran tests to illustrate the workflow change. This is looking ship-shape so marking as such.

Note: See TracTickets for help on using tickets.