Make WordPress Core

Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#43940 closed defect (bug) (fixed)

Privacy Policy Settings: Don't show page selector if you don't have any pages

Reported by: melchoyce's profile melchoyce Owned by: sergeybiryukov's profile SergeyBiryukov
Milestone: 4.9.6 Priority: normal
Severity: normal Version:
Component: Privacy Keywords: gdpr needs-testing has-patch
Focuses: ui, administration Cc:

Description

In core dev chat today, it came up that if you don't have pages on your site, the page selection dropdown is hidden: https://wordpress.slack.com/archives/C02RQBWTW/p1525294644000168

However, if you don't have any pages, we should remove that entire first line and only show the option to create a new page.

Attachments (13)

43940.diff (1.9 KB) - added by abdullahramzan 6 years ago.
Privacy Policy Page.png (33.7 KB) - added by abdullahramzan 6 years ago.
43940-1.diff (1.9 KB) - added by abdullahramzan 6 years ago.
Replace with "There are no pages:"
43940-2.diff (1.9 KB) - added by abdullahramzan 6 years ago.
Remove the Colon ':' and add the Dot '.'
43940-3.diff (2.4 KB) - added by abdullahramzan 6 years ago.
Patch refreshed
1.png (43.9 KB) - added by abdullahramzan 6 years ago.
2.png (42.3 KB) - added by abdullahramzan 6 years ago.
43940-4.diff (2.4 KB) - added by abdullahramzan 6 years ago.
Add Colon ':'
3.png (43.2 KB) - added by abdullahramzan 6 years ago.
Add Colon ':'
Screen Shot 2018-05-10 at 3.13.02 PM.png (65.0 KB) - added by desrosj 6 years ago.
Screen Shot 2018-05-10 at 3.13.13 PM.png (59.0 KB) - added by desrosj 6 years ago.
43940.2.diff (2.5 KB) - added by desrosj 6 years ago.
43940.3.diff (3.4 KB) - added by xkon 6 years ago.
minor CS fixes

Download all attachments as: .zip

Change History (35)

#1 @abdullahramzan
6 years ago

@melchoyce,

Yeah, I was also in the meeting. I am updating the patch for this ticket now.

Thanks

@abdullahramzan
6 years ago

#2 @abdullahramzan
6 years ago

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

@melchoyce,

cc: @desrosj

I have added patch and it looks good in screenshot now.

#3 @desrosj
6 years ago

  • Milestone changed from Awaiting Review to 4.9.6

I'd like to tweak the wording here. What about There are no pages. The create new page is already stated in the button. @allendav, what do you think?

#4 @xkon
6 years ago

Related to the flow of the page #43936 : Specifically https://core.trac.wordpress.org/ticket/43926#comment:4 proposal follows the same principle of showing/hiding the Create button if a Policy and not generally a page page exists or not.

Note that this was discussed before and we reached a decision of showing the create button always as it was hard for a user to understand that he had to 'clear' the select to make the button appear so it was confusing. ( just mentioning )

Last edited 6 years ago by xkon (previous) (diff)

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


6 years ago

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


6 years ago

#7 @allendav
6 years ago

"There are no pages" wfm.

@abdullahramzan
6 years ago

Replace with "There are no pages:"

This ticket was mentioned in Slack in #gdpr-compliance by abdullahramzan. View the logs.


6 years ago

@abdullahramzan
6 years ago

Remove the Colon ':' and add the Dot '.'

#9 @melchoyce
6 years ago

What if we didn't include any text, just the "Create New Page" button? Does that seem confusing or unclear?

#10 @abdullahramzan
6 years ago

  • Keywords 2nd-opinion added

@melchoyce ,

Can we have second opinion or just go without text so I can update the patch accordingly?

#11 @desrosj
6 years ago

  • Keywords needs-refresh added; 2nd-opinion removed

I think it should be there, in my opinion. The text before it says "Select a Privacy Policy page". I think it feels weird if there is no input and no message. I think it will also be rare someone does not see a dropdown.

In addition, here is some feedback on 43940-2.diff:

  • Please use tabs to indent instead of spaces.
  • There should be spaces before and after variables in function calls, after control structures. Example: if( !empty($page_list) ){ should be if ( ! empty( $page_list ) ) {
  • Arrays with values on multiple lines should be aligned at the =>
  • I think the "Or create a new page" text can go away. That is implied by the button text. If anything, it could read just "or"

@abdullahramzan
6 years ago

Patch refreshed

#12 @abdullahramzan
6 years ago

  • Keywords needs-refresh removed

@desrosj ,

Thanks for your feedback.

  • Indentation was due to the editor. I have changed the editor and it looks better now.
  • Updated "Or create a new page" to "or"

@abdullahramzan
6 years ago

@abdullahramzan
6 years ago

#13 @melchoyce
6 years ago

Thanks @desrosj.

@abdullahramzan: one more minor thing — if we're using : in the first line, we should also do Or: on the second line.

Otherwise, looks good to me.

@abdullahramzan
6 years ago

Add Colon ':'

@abdullahramzan
6 years ago

Add Colon ':'

#14 @abdullahramzan
6 years ago

I have updated the patch again as per the above comment by @melchoyce.

@desrosj you can pick the patch which you feel the right one either 43940-3.diff or 43940-4.diff

Thanks

#15 @seusmaniqbal
6 years ago

@abdullahramzan

Tested, works fine for me

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


6 years ago

@desrosj
6 years ago

This ticket was mentioned in Slack in #gdpr-compliance by desrosj. View the logs.


6 years ago

@xkon
6 years ago

minor CS fixes

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


6 years ago

#19 @xkon
6 years ago

43940.2.diff applies and works well.

43940.3.diff has some minor code styling fixes but not something super important I just made a pass since I opened it :)

#20 @SergeyBiryukov
6 years ago

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

In 43238:

Privacy: On Privacy Settings screen, check if any pages exist before displaying the page selector.

Props abdullahramzan, desrosj, melchoyce.
Fixes #43940.

#21 @SergeyBiryukov
6 years ago

In 43239:

Privacy: On Privacy Settings screen, check if any pages exist before displaying the page selector.

Props abdullahramzan, desrosj, melchoyce.
Merges [43238] to the 4.9 branch.
Fixes #43940.

#22 @desrosj
6 years ago

  • Component changed from General to Privacy

Moving to the new Privacy component.

Note: See TracTickets for help on using tickets.