WordPress.org

Make WordPress Core

Opened 4 months ago

Closed 6 weeks ago

Last modified 6 weeks 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 4 months ago.
41603-after-patch.png (171.3 KB) - added by desrosj 4 months ago.
41603.diff (2.4 KB) - added by desrosj 4 months ago.
41603-no-spinner.mp4 (955.0 KB) - added by Presskopp 6 weeks ago.
41603.2.diff (2.6 KB) - added by afercia 6 weeks ago.

Download all attachments as: .zip

Change History (25)

@desrosj
4 months ago

#1 @afercia
4 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
4 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 4 months ago by desrosj (previous) (diff)

#3 @afercia
4 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
3 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
2 months ago

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

#6 @melchoyce
2 months ago

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

#7 @melchoyce
2 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.


6 weeks ago

#9 @Presskopp
6 weeks ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

Spinner is no more working for me

#11 @afercia
6 weeks 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 6 weeks ago by afercia (previous) (diff)

@afercia
6 weeks ago

#12 @afercia
6 weeks 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
6 weeks ago

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

https://cldup.com/_G94FuWkKP.png

#14 @melchoyce
6 weeks ago

Patch works as expected 👍

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


6 weeks ago

#16 @helen
6 weeks ago

  • Keywords commit added

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

#17 @westonruter
6 weeks ago

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

#18 @westonruter
6 weeks ago

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

#19 @westonruter
6 weeks 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
6 weeks 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.