WordPress.org

Make WordPress Core

Opened 12 months ago

Last modified 32 hours ago

#49264 reviewing enhancement

Consider a redesign for the Policy Settings page (includes Policy Guide and future Disclosures & Policies)

Reported by: xkon Owned by: xkon
Milestone: 5.7 Priority: normal
Severity: normal Version:
Component: Privacy Keywords: has-patch 2nd-opinion needs-docs needs-dev-note settings-api needs-testing
Focuses: ui, accessibility, administration Cc:

Description (last modified by xkon)

After some issues on various tickets similar to #44621 that touch various areas as accessibility, design & general functionality of the page ( i.e. to easily mark which text has been updated ).

I would like us if possible to re-think and re-design these pages, that I still feel like it's just a big blob of text. I'll be more than happy to do it and help and see this through as it's been bugging me since 4.9.6.

Some of the early mockups that I had for 4.9.6 were using accordions on this page similar to Site Health & Gutenberg Panels. At that time those had been turned down due to accessibility and other various issues as we wanted to catch the release and be on time and then got kinda forgotten /shrug :D.

We're passed that point now I believe and we're using accordion-type boxes almost everywhere within the admin (widgets, site health, editor, postboxes so on so forth).

This page on a site that has over 10 plugins is seriously a long mess IMHO and very hard to read/keep track at, we're talking about infinite scrolling :D.

In my opinion, we should re-explore the accordion view "per plugin" (similar to how Site Health pages have per check).

This would eventually ( for Policy Guide ):

  1. Greatly reduce scrolling (yes we have a TOC but..ref on the attached screenshot & imagine a multisite with 60+ plugins :) ).
  2. Will be easier to show which plugins have updated text.
  3. Provide better handling on incoming text from plugins/themes and acting as needed for Suggested text and general Guide text.

And other things that will eventually be adjusted on how this whole page works in general.

I'm adding a needs-design but I will be adding a patch also asap (after 5.4) trying to re-create my original idea with the styles that we're currently using for the sake of conversation & checking :).

Attachments (13)

policy_page_9_plugins.png (1.8 MB) - added by xkon 12 months ago.
preview of policy guide with 9 plugins active
49264.diff (11.9 KB) - added by xkon 12 months ago.
49264_preview.jpg (129.9 KB) - added by xkon 12 months ago.
49264.2.diff (26.6 KB) - added by xkon 12 months ago.
49264.2_preview.gif (3.3 MB) - added by xkon 12 months ago.
49264.3_preview.PNG (77.9 KB) - added by xkon 12 months ago.
49264.3.diff (29.1 KB) - added by xkon 12 months ago.
49264.4.diff (28.1 KB) - added by xkon 12 months ago.
49264.5.diff (51.9 KB) - added by xkon 12 months ago.
49264.5_preview.gif (8.6 MB) - added by xkon 12 months ago.
49264.6.diff (52.6 KB) - added by xkon 2 days ago.
refresh
49264.7.diff (55.7 KB) - added by xkon 33 hours ago.
49264.7_preview.gif (7.9 MB) - added by xkon 32 hours ago.

Change History (38)

@xkon
12 months ago

preview of policy guide with 9 plugins active

#1 @xkon
12 months ago

  • Description modified (diff)
  • Summary changed from Consider a redesign for the Policy Guide page to Consider a redesign for the Policy Settings page (includes Policy Guide and future Disclosures & Policies)

@xkon
12 months ago

@xkon
12 months ago

#2 @xkon
12 months ago

  • Keywords needs-design-feedback has-patch 2nd-opinion added; needs-patch needs-design removed

49264.diff makes a start to re-work the Privacy Settings page using a similar style to Site Health. I'll continue working on this and the next patch will hopefully include a full re-work on the Policy Guide.

The "Disclosures & Permissions" was just added for preview purposes even though it doesn't yet exist in Core ( it will be removed on next patch 😊 ).

@xkon
12 months ago

#3 @xkon
12 months ago

49264.2.diff goes further down the changes and "merges" the Policy Guide page with Privacy Settings as a tab. This also makes it a page that it's not hidden anymore as the menu will continue to stay open (this is currently an issue as the privacy-policy-guide.php has no parent/menu so there's no indication at all).

There are various more little things to do here especially regarding the policy text loops and how we handle the suggested texts etc, but I think that the base is there now 😅.

As you can see (in my opinion) even though the text might still be long to read, the page is already becoming way more manageable and you can also keep your focus on the open policy that you're reading and not get lost from scrolling so easily.

Note that this patch introduces a new css file called privacy-settings.css instead of continuing inside the older edit.css that we are using until now.

Preview at 49264.2_preview.gif 😊 .

I'll continue the updates here asap, but feel free to jump in with any comments or ideas!

#4 @xkon
12 months ago

49264.3.diff adds finishing touches to the CSS while moving things around to properly work (non-breaking changes) and all the main functionality is patched now 🙂.

It also moves & adds any extra CSS in common.css to avoid creating new files, so it makes it non-depended on major releases only.

Next steps:

  1. Wait for feedback.
  2. Work on the wp_add_privacy_policy_content() that requires a string and there have been a lot of plugins that are not adding styles, headers, etc correctly. I will be seeing on updating the function if possible to standardize a little bit how Policies can be added via an array as well (to keep back-compatibility also).

With an array, we can have more control over the output HTML that will help to prevent false classes and headings as well as make the Copy actions easier.

As an example we can have an array like:

<?php

$policy = array(
	array(
		'title'          => 'What personal data we collect and why we collect it',
		'tutorial_text'  => array(
			'Line 1',
			'Line 2',
			'Line 3',
			'Line 4',
		),
		'suggested_text' => array(
			'Write this',
			'Write this as well',
		),
	),
	array(
		'title'          => 'Embedded content from other websites',
		'tutorial_text'  => array(
			'Line 5',
			'Line 6',
			'Line 7',
		),
		'suggested_text' => array(
			'Write that',
			'Write that as well',
		),
	),
);

And output it's contents like so:

foreach ( $policy as $section ) {
	echo $section['title'];
	if ( array_key_exists( 'tutorial_text', $section ) ) {
		foreach ( $section['tutorial_text'] as $text ) {
			echo '<p class="privacy-policy-tutorial">' . $text . '</p>';
		}
	}
	if ( array_key_exists( 'suggested_text', $section ) ) {
		echo '<p class="privacy-policy-suggested-text">' . __( 'Suggested text:' ) . '</p>';
		foreach ( $section['suggested_text'] as $text ) {
			echo '<p class="privacy-policy-suggested-text">' . $text . '</p>';
		}
	}
}

@xkon
12 months ago

@xkon
12 months ago

#5 @xkon
12 months ago

49264.4.diff makes some minor edits for a clean application on top of 49264.3.diff.

I'll start working on the array proposal of comment 4 and will split that into a different ticket that will need this applied as well.

Since this has a lot of moving parts due to re-organizing the page I prefer to split them in smaller chunks regarding functionality and different things that will need to be altered.

@xkon
12 months ago

#6 @xkon
12 months ago

  • Keywords needs-docs needs-dev-note added

After going over and over iterating on the array of my previous comments, I realized that there's no point of changing the current functionality and introduce back-compat issues "just for a text". Also if any author wanted multidimensional nesting it would be hard to follow after a point.

49264.5.diff takes a different approach on the Policy text by introducing just another "needed" class. At the moment the "Suggested" text was not using any class at all so it was hard to define and target it for easier usage either for CSS or copy/paste reasons etc.

Instead we can now use 2x classes. privacy-policy-tutorial (existing for general text) and the new privacy-policy-suggested-text for any suggestion related text.

I've also altered the get_default_content() to split our default policy text into an actual Guide and also as an incoming suggestion. This makes it easier to understand as you're not seeing everything in 1 read from Core and you can simply read the Guide only if you need to do so.

In this patch wp_get_default_privacy_policy_content filter is also getting deprecated. I'm not entirely sure why this was added but unfortunately I've seen a couple of plugins hooking there instead of using the correct wp_add_privacy_policy_content, resulting on changing/extending the Core guide + suggestions. Which IMHO is wrong and shouldn't happen. The guide and core suggestions should only be Cores responsibility with no way of altering them.

Preview at 49264.5_preview.gif .

@azaozz, this is a patch for a future release (if accepted) so it can definitely wait for now, but I'd love to hear your thoughts and any pointers before continuing especially regarding the deprecation, whenever you find some time :-).

--

Todo:
If/when this patch is implemented we will have to add some extra CSS rules for back-compat with the existing ones and update the Copy to clipboard functionality to support both if possible.

Side note:
It seems that we totally missed of informing the Authors about how to properly write their Policy in general as there's no mention at all about classes and correct text convention at https://developer.wordpress.org/plugins/privacy/suggesting-text-for-the-site-privacy-policy/ .

We should be updating those docs in any case and explaining that it would be for the best to start with <h2> and smaller headings, as well as regarding the correct <p> and their classes.

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


12 months ago

#8 @xkon
12 months ago

#46687 was marked as a duplicate.

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


8 months ago

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


4 months ago

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


4 months ago

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


4 months ago

#14 @garrett-eclipse
4 months ago

  • Keywords settings-api added

#15 @hedgefield
3 months ago

  • Keywords needs-design-feedback removed

Hey @xkon, stellar work on wrangling this page into something scannable! As the person who designed the accordions for Site Health I have to agree with using that solution here also ;) I think the accessibility concerns you mentioned mostly had to do with making sure the accordion is technically a button and things like that, right? Which I'm sure have indeed been solved by the evidence of its inclusion in other places of the admin.

As a designer I cannot judge the code of this patch but it looks like you did a thorough job of thinking this through. What's the state of the patch currently? Could it be included in 5.6? Any points of discussion that you've become aware of in the meantime that we need to resolve? Do you need feedback or signoff from someone? I'd love to get this shipped. Let me know.

#16 @garrett-eclipse
3 months ago

  • Focuses privacy removed

Dropping privacy focus as it's already in the Privacy component.

#17 @xkon
2 months ago

Heya @hedgefield, thank you for the kind words & sorry for the late reply but some personal things have been keeping me off contributions, unfortunately.

To give an overview here and some thoughts as a tl;dr (since it's been ~9 months and everything was in my head for this task):

  1. I had discussed with @Clorith and we had agreed that maybe we should make the Site Health CSS "global" so it can be shared on other pages also if need be instead of re-using essentially the same rules but with different names (as was done in these patches). Not sure if a change like that actually happened in the meantime.
  1. Notable changes that should be checked & devs should be informed beforehand & have time to update their code before this lands are the function changes for a proper CSS output see comment:4 as an example and the "side note" at the end of comment:6.
  1. The "copy to clipboard" functionality would need some re-work to be able to catch properly the new output.

I'll try to find some time to at least re-apply the patch and see what has changed since then, in case it needs a refresh etc, but I can't promise anything solid at the moment so if anyone from #core-privacy wants to take ownership & jump in (i.e. @garrett-eclipse 👋) it would be great!.

#18 @garrett-eclipse
2 months ago

  • Milestone changed from Future Release to 5.7
  • Owner changed from xkon to garrett-eclipse
  • Status changed from assigned to reviewing

Hey @xkon, all the best to you and yours.
I'd been meaning to dive into this one for 5.6 but didn't have the bandwidth. I've owned it and milestoned for 5.7. I'll dive in and start iterating when I have a chance.

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


7 weeks ago

#20 @hellofromTonya
2 weeks ago

#52087 was marked as a duplicate.

This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.


8 days ago

@xkon
2 days ago

refresh

#22 @xkon
2 days ago

49264.6.diff refreshes previous patch to apply cleanly.

Summarizing so we can keep this moving: The idea is to beautify the pages and make a seamless transition between the "Settings" & the Policy Guides page. This is easily done by using a similar design to Site Health. Due to the tabbed menu in the future we will also be able to add more pages like such as Disclosures etc if need be and keep the main menu simple by having everything under "Privacy".

---

QA/Testing scope & expected results.

Essentially it's a full re-design that uses Tabs & Accordions with a similar layout to Site Health.

The Settings page has some minor change to add the needed header + tabs and the Policy Guide page is now included here also with a new design (instead of the long-text current page).

Expected results of applying the patch are:

  1. Layout as shown in 49264.5_preview.gif.
  2. Accordion in Policy Guide page should be a11y friendly (since we're using the same accordions as Site Health pages everything should already be covered).

Notes:
If you are using plugins/themes on your testing their Policy Guide texts might not render properly since they will have the new introduced class missing. This will be covered when this is finalized and included in Dev Notes.

The "copy to clipboard" button will be worked on a different ticket to avoid a bigger scope here.

Last edited 2 days ago by xkon (previous) (diff)

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


2 days ago

#24 @hellofromTonya
2 days ago

  • Keywords needs-testing added

Discussions in slack about current state and next steps for this ticket.

  • Patch is ready for 2nd look. @azaozz can you look at the patch please?
  • @xkon will open a new ticket to split out the work for the "copy to clipboard".
  • Ready for testing.

@xkon notes:

Maybe @azaozz will be able to help at some point for that 2nd look since he was fully involved on the original policy texts as well so he knows his way around there

@xkon please add the following info to help the QA/Testing folks:

  • Steps to test it
  • What the expected results/behavior should be

@xkon
33 hours ago

@xkon
32 hours ago

#25 @xkon
32 hours ago

  • Owner changed from garrett-eclipse to xkon

@hellofromTonya I added 49264.7.diff that includes the Copy button changes also. They where simple so splitting it up was going to make testing a bit harder as 2 patches would need to be applied for a simple change.

To recap for a patch review & testing:

49264.7.diff include all necessary changes for a re-design of the Privacy related pages ( including Settings & Policy Guide ). Using the same page styling that we used on Site Health we can now have a more compact and easy way to move around policies and a nicer Settings page overall.

By applying the patch the end result should be similar to 49264.7_preview.gif .

All actions should work fine ( select policy page, create new page, copy to clipboard ).

To make the new policy text styling work the Copy action and the way that the incoming policies are added via filters need to change a bit ( basically by adding 1 new CSS style ).

Dev Notes: Example Policy ( snippet taken from WooCommerce ) as a "current" vs "new" example to understand the differences and changes needed.

  • wp-suggested-text wrapper is not needed any more
  • privacy-policy-tutorial is not used any more.
  • Instead we're focusing on privacy-policy-suggested-text to stylize the suggested text properly & copy it to clipboard.
  • privacy-policy-suggested-text-header is also used but it's not mandatory.

Current styling of a policy:

$content = '<div class="wp-suggested-text">' .
	'<p class="privacy-policy-tutorial">' .
		__( 'This sample language includes the basics around what personal data your store may be collecting, storing and sharing, as well as who may have access to that data. Depending on what settings are enabled and which additional plugins are used, the specific information shared by your store will vary. We recommend consulting with a lawyer when deciding what information to disclose on your privacy policy.', 'woocommerce' ) .
	'</p>' .
	'<p>' . __( 'We collect information about you during the checkout process on our store.', 'woocommerce' ) . '</p>' .
	'<h2>' . __( 'What we collect and store', 'woocommerce' ) . '</h2>' .
	'<p>' . __( 'While you visit our site, we’ll track:', 'woocommerce' ) . '</p>' .
	'<ul>' .
		'<li>' . __( 'Products you’ve viewed:  we’ll use this to, for example, show you products you’ve recently viewed', 'woocommerce' ) . '</li>' .
		'<li>' . __( 'Location, IP address and browser type: we’ll use this for purposes like estimating taxes and shipping', 'woocommerce' ) . '</li>' .
		'<li>' . __( 'Shipping address: we’ll ask you to enter this so we can, for instance, estimate shipping before you place an order, and send you the order!', 'woocommerce' ) . '</li>' .
	'</ul>' .
	'</div>';

New styling needed after patch is applied:

The privacy-policy-suggested-text can be applied to a <p> or a <div> etc to create the indentation needed.

$content = '<p>' .
		__( 'This sample language includes the basics around what personal data your store may be collecting, storing and sharing, as well as who may have access to that data. Depending on what settings are enabled and which additional plugins are used, the specific information shared by your store will vary. We recommend consulting with a lawyer when deciding what information to disclose on your privacy policy.', 'woocommerce' ) .
	'</p>' .
	'<p>' . __( 'We collect information about you during the checkout process on our store.', 'woocommerce' ) . '</p>' .
	'<h2>' . __( 'What we collect and store', 'woocommerce' ) . '</h2>' .
	'<p class="privacy-policy-suggested-text-header">' . __( 'Suggested text:' ) . '</p>' .
	'<div class="privacy-policy-suggested-text">' .
	'<p>' . __( 'While you visit our site, we’ll track:', 'woocommerce' ) . '</p>' .
	'<ul>' .
		'<li>' . __( 'Products you’ve viewed:  we’ll use this to, for example, show you products you’ve recently viewed', 'woocommerce' ) . '</li>' .
		'<li>' . __( 'Location, IP address and browser type: we’ll use this for purposes like estimating taxes and shipping', 'woocommerce' ) . '</li>' .
		'<li>' . __( 'Shipping address: we’ll ask you to enter this so we can, for instance, estimate shipping before you place an order, and send you the order!', 'woocommerce' ) . '</li>' .
	'</ul>' .
	'</div>';
Note: See TracTickets for help on using tickets.