WordPress.org

Make WordPress Core

Opened 11 months ago

Closed 8 months ago

Last modified 8 months ago

#41603 closed defect (bug) (fixed)

Custom date format field too small on mobile

Reported by: desrosj Owned by: westonruter
Milestone: 4.9 Priority: normal
Severity: normal Version: trunk
Component: Date/Time Keywords: dev-reviewed has-patch commit
Focuses: ui, administration Cc:

Description

On Settings > General, there is a field for entering a custom PHP date format string. This field is too small on mobile. The date preview is also broken up into two lines.

In #34539, the narrow field issue was addressed in other locations. Increasing the max-width here (it has its own CSS definition) does not necessarily fix the problem for this field.

The preview changes every time the option is changed, but it is displayed next to the custom option. If we move the preview to its own line, it fixes the issue of the preview being split up, and allows the field to be widened quite a bit.

Attachments (5)

41603-before-patch.png (124.5 KB) - added by desrosj 11 months ago.
41603-after-patch.png (171.3 KB) - added by desrosj 11 months ago.
41603.diff (2.4 KB) - added by desrosj 11 months ago.
41603-no-spinner.mp4 (955.0 KB) - added by Presskopp 8 months ago.
41603.2.diff (2.6 KB) - added by afercia 8 months ago.

Download all attachments as: .zip

Change History (25)

@desrosj
11 months ago

#1 @afercia
11 months ago

To clarify: the bold text "Preview:" would be visible also on large screens. Not that I'm opposed to that :) a11y-wise, visible text is always better then visually hidden one. Just thought was something to clarify.

#2 @desrosj
11 months ago

@afercia Thanks for clarifying! I forgot to mention that this change was for all orientations. I also was not sure if there are a11y benefits to putting the preview at the top as opposed to the bottom, or if there are any tags that we should use other than <span> for something like this. Would a <label> be beneficial?

Last edited 11 months ago by desrosj (previous) (diff)

#3 @afercia
11 months ago

@desrosj thanks! Hm, no I think that text doesn't need a label and it's fine it's to the bottom. FWIW the settings pages would be greatly improved for accessibility and that's one of the goals of the Settings API project (just an experiment for now, nothing official).

#4 @SergeyBiryukov
10 months ago

  • Component changed from General to Date/Time
  • Focuses administration added
  • Milestone changed from Awaiting Review to 4.9

Let's handle this along with #34539.

#5 @melchoyce
9 months ago

#34539 is merged. What do we need to get this finished up?

#6 @melchoyce
9 months ago

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

#7 @melchoyce
9 months ago

  • Resolution set to fixed
  • Status changed from assigned to closed

In 41857:

Settings: Widen time/day input fields on General Settings page, and move the preview onto the next line.

Props desrosj, afercia.
Fixes #41603.

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


8 months ago

#9 @Presskopp
8 months ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Spinner is no more working for me

#11 @afercia
8 months ago

  • Keywords needs-patch added; has-patch removed
  • Version set to trunk

The JS part to update the preview and display the spinner looks for a sibling element with class .example: .siblings( '.example' )

This change introduces a paragraph so the example is not a sibling any longer and the JS part fails.

Last edited 8 months ago by afercia (previous) (diff)

@afercia
8 months ago

#12 @afercia
8 months ago

  • Keywords has-patch added; needs-patch removed

41603.2.diff updates the jQuery selectors used in the JS part to make the preview and spinner work again.

#13 @afercia
8 months ago

Of course, the spinners will still be displayed on the right of the preview:

https://cldup.com/_G94FuWkKP.png

#14 @melchoyce
8 months ago

Patch works as expected 👍

This ticket was mentioned in Slack in #core-committers by melchoyce. View the logs.


8 months ago

#16 @helen
8 months ago

  • Keywords commit added

That chaining is getting a little out of control but patch looks good to me. :)

#17 @westonruter
8 months ago

  • Owner changed from melchoyce to westonruter
  • Status changed from reopened to reviewing

#18 @westonruter
8 months ago

  • Keywords dev-reviewed added; dev-feedback needs-testing removed

#19 @westonruter
8 months ago

  • Resolution set to fixed
  • Status changed from reviewing to closed

In 42134:

Settings: Fix date/time format previewing.

Props afercia.
Amends [41857].
Fixes #41603.

#20 @westonruter
8 months ago

In 42135:

Settings: Fix date/time format previewing.

Props afercia.
Amends [41857].
Fixes #41603 for 4.9.

Note: See TracTickets for help on using tickets.