#49264 closed enhancement (fixed)
Redesign the Privacy Settings page (includes Privacy Policy Guide)
Reported by: | xkon | Owned by: | xkon |
---|---|---|---|
Milestone: | 5.7 | Priority: | normal |
Severity: | normal | Version: | |
Component: | Privacy | Keywords: | has-patch has-screenshots commit |
Focuses: | ui, accessibility, administration | Cc: |
Description (last modified by )
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 ):
- Greatly reduce scrolling (yes we have a TOC but..ref on the attached screenshot & imagine a multisite with 60+ plugins :) ).
- Will be easier to show which plugins have updated text.
- 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 (16)
Change History (68)
#1
@
5 years 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)
#2
@
5 years 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 😊 ).
#3
@
5 years 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
@
5 years 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:
- Wait for feedback.
- Work on the
wp_add_privacy_policy_content()
that requires astring
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>'; } } }
#5
@
5 years 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.
#6
@
5 years 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.
5 years ago
This ticket was mentioned in Slack in #core-privacy by burtrw. View the logs.
5 years ago
This ticket was mentioned in Slack in #design by garrett-eclipse. View the logs.
4 years ago
This ticket was mentioned in Slack in #design by garrett-eclipse. View the logs.
4 years ago
This ticket was mentioned in Slack in #design by karmatosed. View the logs.
4 years ago
#15
@
4 years 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
@
4 years ago
- Focuses privacy removed
Dropping privacy
focus as it's already in the Privacy
component.
#17
@
4 years 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):
- 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.
- 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.
- 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
@
4 years 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.
4 years ago
This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.
4 years ago
#22
@
4 years 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:
- Layout as shown in 49264.5_preview.gif.
- 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.
This ticket was mentioned in Slack in #core-privacy by hellofromtonya. View the logs.
4 years ago
#24
@
4 years 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
#25
@
4 years 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 moreprivacy-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>';
#26
@
4 years ago
Is there chance of getting final feedback on this done this week so we can ship it in 5.7 beta 1?
#27
@
4 years ago
@SergeyBiryukov , would you say it's ok to inform plugin authors for the changes needed for the CSS as shown in comment 25 ?
That's what the "2nd-opinion" tag was on this ticket for basically as I couldn't find any possible way of backwards compatibility unfortunately since we where using a different approach on styles until now.
It's the only way imho to have a nice and easy to read styling across the board of incoming policies without making further adjustments and forcing arrays etc (which completely changes the logic), it's the "least impact" in regards to changes.
If you think that's ok then the patch should be ok for a final test & commit.
This ticket was mentioned in Slack in #accessibility by joedolson. View the logs.
4 years ago
This ticket was mentioned in Slack in #core by xkon. View the logs.
4 years ago
This ticket was mentioned in Slack in #accessibility by ryokuhi. View the logs.
4 years ago
#31
@
4 years ago
- Keywords needs-refresh added; 2nd-opinion needs-docs needs-dev-note settings-api needs-testing removed
- Summary changed from Consider a redesign for the Policy Settings page (includes Policy Guide and future Disclosures & Policies) to Redesign the Privacy Settings page (includes Privacy Policy Guide)
This needs refresh again :/. I'm working on a new PR already for it.
I'm also adjusting all the tags here and will re-add whatever is necessary after the PR is ready for testing as due to some other commits there's a chance that we can skip introducing any breaking changes here as well.
This ticket was mentioned in PR #957 on WordPress/wordpress-develop by mrxkon.
4 years ago
#32
- Keywords needs-refresh removed
- Updates the design for Settings -> Privacy
- Updates the design for Privacy Policy Guide
- Splits the WordPress Policy into 2 texts, a guide and the suggested policies for clarity.
- Updates necessary css
- Updates necessary JS for Copy suggested text functionality
Trac ticket: https://core.trac.wordpress.org/ticket/49264
#33
@
4 years ago
- Keywords needs-testing has-screenshots added
:wave: @hellofromTonya & @francina . As discussed via Slack on previous chats, I've updated the PR so the patch should be applying cleanly now. I've also re-written some parts of CSS since other commits have been made which gives us the ability to have back-compat also so nothing should be breaking & no need for extra docs or dev-notes :D.
To test:
- Apply the patch provided from the Github PR.
- Install WooCommerce for example (to have an extra policy text).
- Make sure TwentyTwentyOne is selected (to have an extra policy text).
Step 1 Settings tab:
- Go to Settings -> Privacy (Settings Tab is selected by default), you should see the new design as shown in 49264.5_preview.gif
- The Settings options ( Create a new Privacy Policy Page / Change your Privacy Policy page ) should be properly working.
- The links on the Settings page are going to the correct Policy page (Edit or preview links ) are redirecting to the correct Policy page that is set.
- The link Check out our Privacy Policy guide should point to the Policy Guide tab.
Step 2 Policy Guide tab:
- Go to Policy Guide tab, the design should again be similar to the preview mentioned above.
- The accordions should be accessible by keyboard as well (similar to Site Health).
- The Privacy Policy Guide, only contains the "guide" section of our older text.
- The WordPress under Policies only contains all of the Core suggested policy texts.
The design idea here (just in case you want to test more plugins for compatibility) is that you have a general text shown normally and all of the "suggested text" is marked with the gray border on the left side to have a visual difference.
- Press the Copy suggested policy text to clipboard button to copy the suggested text. The copied text should only include any titles (h1,h2,h3 etc) as well as any text that has the gray border.
---
I hope that the above steps are clear just in case we still have time to get this in. Thanks!
This ticket was mentioned in Slack in #core by paaljoachim. View the logs.
4 years ago
#35
@
4 years ago
It was somewhat difficult getting the testing commands correct.
- Applied patch from trac ticket.
npm run grunt patch:49264
Then
npm run patch https://github.com/WordPress/wordpress-develop/pull/957
I muddled a bit back on forth. In the end it looks like I got it working.
EDIT: Somewhere with the above code I need to include
npm run build
to sure of the command order though.
That is probably why the accordions are not working.
I will update the above commands when I get it right.
Testing environment: Chrome, localhost:8889, OSX 10.15.7 (Catalina).
The testing.
EDIT: There is a lot of feedback here to various things I discovered along the way.
1.Apply the patch provided from the Github PR.
->Done.
2.Install WooCommerce for example (to have an extra policy text).
->Done.
3.Make sure TwentyTwentyOne is selected (to have an extra policy text).
->Done.
Step 1 Settings tab:
1.Go to Settings -> Privacy (Settings Tab is selected by default), you should see the new design as shown in 49264.5_preview.gif
-> Yes. What I see is very similar. I have just not yet pressed the Create button to
"Create a new Privacy Policy Page"
Below it I see the
"Change your Privacy Policy page" = should be a capital P. A drop down next to it with various options. Most of the options are from WooCommerce. I am able to select the various options. I have not yet clicked "Create" or "Use this Page" buttons.
2.The Settings options ( Create a new Privacy Policy Page / Change your Privacy Policy page ) should be properly working.
-> Clicking the "Create" button. The screen jumped to a new(?) Privacy Policy page. With various information added to the Classic Editor. So that I will need to if using Gutenberg click each area and then click Convert to blocks. I went ahead and converted all of these to blocks. At the top is a notification that says:
"Need help putting together your new Privacy Policy page? Check out our guide for recommendations on what content to include, along with policies suggested by your plugins and theme." Here it would be nice if the sentence began with a Do you....then the rest of the information. Such as: Do you need help putting together....
A link to: "View Privacy Policy Guide" To open it in a new tab I would have to right click and select to open it in a new tab. Ahhh the link brings me back to the Settings -> Privacy -> Policy Guide tabbed area. That was not clear. I thought it was to an external site.
In a new tab I am at the Policy Guide. Clicking the Settings tab. I was expecting to see a mention that I have already created a Privacy Policy Page. As I can create a bunch of Policy pages. I notice these show up in the drop down right below.
I selected in the drop down one of the Privacy Policy pages that I made, and clicked "Use This Page". A notification showed up just above the Privacy Settings saying: "Privacy Policy page updated successfully." = Remember capital P in Page. (Or change "
Create a new Privacy Policy Page" Page = P to a smaller case page.)
Selected Cart in the drop down. Clicked "Use This Page" Notification says:
"Privacy Policy page setting updated successfully. Remember to update your menus!"
--
Bottom line for the Settings -> Privacy Settings.
Creating a new Privacy Policy Page/page works well!
Changing the Privacy Policy page/Page works well!
-
-
3.The links on the Settings page are going to the correct Policy page (Edit or preview links ) are redirecting to the correct Policy page that is set.
--> Yes. Clicking Edit or view goes to the Privacy Policy page that is set in the drop down further below.
4.The link Check out our Privacy Policy guide should point to the Policy Guide tab.
--> Yes. Clicking "Check out our Privacy Policy guide" goes to the Policy Guide tab.
Step 2 Policy Guide tab:
5.Go to Policy Guide tab, the design should again be similar to the preview mentioned above.
--> Yes. The design is similar to the preview mentioned above.
6.The accordions should be accessible by keyboard as well (similar to Site Health).
--> I am able to tab my way through each accordion, and see the blue outline of each.
I am though not able to open these up. I click the arrows in the accordions but nothing happens.
7.The Privacy Policy Guide, only contains the "guide" section of our older text.
--> Ok.
8.The WordPress under Policies only contains all of the Core suggested policy texts.
--> Ok.
After this it gets kinda hazy. Perhaps it has to do with the contents inside the accordions.
This I am not seeing:
"Press the Copy suggested policy text to clipboard button to copy the suggested text."
#36
@
4 years ago
Hey @paaljoachim thanks for all the testing!
--> I am able to tab my way through each accordion, and see the blue outline of each.
I am though not able to open these up. I click the arrows in the accordions but nothing happens.
After this it gets kinda hazy. Perhaps it has to do with the contents inside the accordions.
Yes correct everything else (Copy etc) are inside the accordions. They should be working fine as the JS needed is inside the PR provided. Maybe for some reason the JS file was not getting updated (i.e. cache etc).
I'm usually working & testing directly from /src, so it should be as simple as applying the patch and running an npm run dev
then loading the site ( that's what I do at least when I'm checking out tickets ).
This ticket was mentioned in Slack in #core by paaljoachim. View the logs.
4 years ago
#38
@
4 years ago
I have had to reset using:
git reset --hard
Then the command:
npm run grunt patch:49264
To get JS to work I added from inside the wordpress-develop directory:
npm run build
Even though I added the above command the JS for the accordions did not work in my test for this ticket.
With the above commands I was able to correctly test https://core.trac.wordpress.org/ticket/48562#comment:22
I also added the command:
npm run dev
It seemed to get stuck in terminal at
Running "usebanner:files" (usebanner) task √ grunt-banner completed successfully Running "_watch" task Waiting...
but I logged out hard refreshed in Chrome and then logged in again.
This time JS in the accordion did work. YAhhh!
Continuing the test:
- Press the Copy suggested policy text to clipboard button to copy the suggested text. The copied text should only include any titles (h1,h2,h3 etc) as well as any text that has the gray border.
--> I opened up the Policies and WordPress accordion.
Clicked the button "Copy suggested policy text to clipboard"
Made a new page. Pasted all the content into the page.
All the paragraphs where included like so, but no titles/headings where included.
Our website address is: http://localhost:8889.
When visitors leave comments on the site we collect the data shown in the comments form, and also the visitor’s IP address and browser user agent string to help spam detection.
An anonymized string created from your email address (also called a hash) may be provided to the Gravatar service to see if you are using it. The Gravatar service privacy policy is available here: https://automattic.com/privacy/. After approval of your comment, your profile picture is visible to the public in the context of your comment.
This ticket was mentioned in Slack in #core by paaljoachim. View the logs.
4 years ago
#40
@
4 years ago
Patch from pr:957 applies properly for me, and functions as described. I've not done any checks on the code it self, just looked over that functionality is in place since there was mention of it not working as described.
#41
@
4 years ago
- Keywords commit added; needs-testing removed
- Status changed from reviewing to accepted
Hey @paaljoachim thanks for checking this again.
You mention:
Then the command:
npm run grunt patch:49264
npm run grunt patch:49264
will show the patches uploaded to the ticket. The latest one is the Github PR and you can't test that via that command as it won't list the PR to select it. You can try via npm run grunt patch:https://github.com/WordPress/wordpress-develop/pull/957
if that works for you to test the PR. Just clarifying in case you accidentally checked an older patch :/ .
I've uploaded a preview of the PR at 49264_titles_preview.gif showing that the titles & overall design are working as expected in both Gutenberg & Classic Editor :).
---
Thanks @Clorith for the 2nd eyes as well!
Marking for commit since everything looks good for now and we can adjust further during the Beta as well I suppose if need be.
#42
@
4 years ago
Well that was just awesome!
Open terminal insert
cd wordpress-develop
then
npm run grunt patch:https://github.com/WordPress/wordpress-develop/pull/957
And voila it works. No error code no problem.
#43
@
4 years ago
Reviewed this for accessibility, and it generally seems fine. Two things I'd like to see adjusted:
1) It think it would be clearer if the 'Copy suggested policy' button indicated which policy was being copied, e.g. 'Copy suggested {WordPress} policy statements' (this is not a new issue with this change.)
2) The headings hierarchy is broken, due to the usage of H2 within policy statements. I don't have a great suggestion for that; according to the hierarchy, they should be using H5, so that they're descendants of the policy group. However, they most frequently would need to be H2 when used in the site's policies. Modifying these consistently would be difficult, given that most policy text is externally supplied and not structured by WordPress.
Neither of these are blocking issues.
#44
@
4 years ago
Thanks @joedolson !
- 'Copy suggested {WordPress} policy statements'
I agree, maybe we can address that during beta even (?). Would it make sense to also do the same for the a11y.speak attached to the action as well?
- The headings hierarchy is broken
Unfortunately I'm not sure what's the best approach there either, it will most likely always be broken no matter what we choose for core at least since the incoming policies can add various headings as well.
One way I suppose would be to parse the text before output and actually change them all to H5 as you suggest even if it "breaks" the copied text output on the front-end.
In my experience up to now (from all the policies that I've made) I rarely used the titles since the policy page had it's sections marked up already, so I've been mostly using the suggested text itself only. But that might just be me so it might end up being annoying for others to have to manually change the titles.
We can split this as a follow-up to get some more opinions I guess :) .
preview of policy guide with 9 plugins active