WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 months ago

Last modified 2 months ago

#35776 closed defect (bug) (fixed)

Installation pages do not have forms.css enqueued

Reported by: iseulde Owned by: SergeyBiryukov
Milestone: 5.3 Priority: normal
Severity: normal Version:
Component: Upgrade/Install Keywords: good-first-bug has-patch
Focuses: ui Cc:
PR Number:

Description


Attachments (11)

Screen Shot 2016-02-08 at 16.05.48.png (44.6 KB) - added by iseulde 4 years ago.
with_forms_css.png (13.3 KB) - added by dan@… 4 years ago.
with forms css
without_forms_css.png (11.8 KB) - added by dan@… 4 years ago.
without forms css
35776.patch (2.4 KB) - added by bassgang 4 years ago.
Add form styles to install.css
without-form-styles-before01.jpg (50.6 KB) - added by bassgang 4 years ago.
Install page Step 2 without form styles 01
without-form-styles-before02.jpg (52.8 KB) - added by bassgang 4 years ago.
Install page Step 2 without form styles 02
with-form-styles-after01.jpg (51.0 KB) - added by bassgang 4 years ago.
Install page Step 2 with form styles 01
with-form-styles-after02.jpg (49.5 KB) - added by bassgang 4 years ago.
Install page Step 2 with form styles 02
35776.diff (1.4 KB) - added by cdog 2 years ago.
install step 1 trunk.png (47.9 KB) - added by afercia 2 months ago.
Install screen on current trunk 5.3
install step 1 wp51.png (43.8 KB) - added by afercia 2 months ago.
Install screen on WP 5.1

Download all attachments as: .zip

Change History (27)

#1 @dan@…
4 years ago

  • Keywords 2nd-opinion added

Should it be? When I did enqueue it during the install page formatting got screwed up and i'm not sure I see what the difference is. See the following attachments:

@dan@…
4 years ago

with forms css

@dan@…
4 years ago

without forms css

#2 @dan@…
4 years ago

  • Keywords dev-feedback added

Of course the formatting issue differ depending on the order forms is enqueued, Is there some specific css in forms.css that is needed that can be coped to install.css ?

#3 @SergeyBiryukov
4 years ago

  • Component changed from General to Upgrade/Install
  • Focuses ui added

#4 @bassgang
4 years ago

@iseulde Can you please clarify what exact styles are missing from forms.css.

How should it look like?

#5 @iseulde
4 years ago

  • Keywords 2nd-opinion dev-feedback removed

At first glance it looks like the input styles are missing. You can see the default browser focus style and checkbox style in the first screenshot. If the password input doesn't look good, then it'd need to be adjusted.

#6 @bassgang
4 years ago

Attached is a patch which mirrors relevant styles from "wp-admin/css/forms.css" into "wp-admin/css/install.css".
Find also before and after screenshots attached, the changes are mainly about the checkbox at the bottom of the form.

@bassgang
4 years ago

Add form styles to install.css

@bassgang
4 years ago

Install page Step 2 without form styles 01

@bassgang
4 years ago

Install page Step 2 without form styles 02

@bassgang
4 years ago

Install page Step 2 with form styles 01

@bassgang
4 years ago

Install page Step 2 with form styles 02

#7 @bassgang
3 years ago

  • Keywords has-patch reporter-feedback added

#8 @DrewAPicture
3 years ago

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

Assigning to mark the good-first-bug as "claimed".

See 35776.patch

@cdog
2 years ago

#9 @cdog
2 years ago

See 35776.diff for a simpler approach. It imports core forms.css styles and then adjusts some form controls properties to match the new styles.

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


4 months ago

#11 @SergeyBiryukov
4 months ago

  • Keywords reporter-feedback removed
  • Milestone changed from Future Release to 5.3
  • Owner changed from bassgang to SergeyBiryukov
  • Status changed from assigned to reviewing

#12 @SergeyBiryukov
4 months ago

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

In 45673:

Upgrade/Install: Bring some consistency to installation screen styles.

  • Include forms.css and l10n.css, for consistency with login screen and other admin screens.
  • Remove redundant @import directives from login.css for files already declared as dependencies.
  • Adjust margin on password strength meter for consistency with other fields.
  • Increase font size for "You will need this password to log in" notice.
  • Fix misaligned icon on "Hide" button for the password.

Props iseulde, dan@…, bassgang, cdog, johnbillion, nmenescardi, mukesh27, SergeyBiryukov.
Fixes #35776, #47757, #47758.

#13 @SergeyBiryukov
3 months ago

In 45844:

Upgrade/Install: Bring some consistency to installation screen styles.

  • Include forms.css and l10n.css, for consistency with login screen and other admin screens.
  • Remove redundant @import directives from login.css for files already declared as dependencies.
  • Adjust margin on password strength meter for consistency with other fields.
  • Increase font size for "You will need this password to log in" notice.
  • Fix misaligned icon on "Hide" button for the password.

Props iseulde, dan@…, bassgang, cdog, johnbillion, nmenescardi, mukesh27, alpipego, SergeyBiryukov.
Merges [45673] to the 5.2 branch.
Fixes #35776, #43483, #47757, #47758.

#14 follow-up: @JeffPaul
3 months ago

@SergeyBiryukov looks like the prop in r45673 and r45844 should have been to danmicamediacom.

#15 in reply to: ↑ 14 @SergeyBiryukov
3 months ago

Replying to JeffPaul:

looks like the prop in r45673 and r45844 should have been to danmicamediacom.

Thanks for bringing that up!

Previously, the rule of thumb was to always use the name that Trac shows:
https://wordpress.slack.com/archives/C18723MQ8/p1471699137000226

However, there are indeed some quirks when the username has spaces or is an email address:
https://wordpress.slack.com/archives/C18723MQ8/p1513306349000049

So apparently user_nicename should be used instead, but it's not currently available for Trac templates.

I'll double check the correct username when updating the Credits API for 5.2.3.

#16 @afercia
2 months ago

Looks like on the install page > step 1, fhe password input field and button need some love in the responsive view. Didn't look great in wp 5.1 as well. See screenshots below.

I think it can be addressed in #42888 together with other improvements. /Cc @SergeyBiryukov

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

@afercia
2 months ago

Install screen on current trunk 5.3

@afercia
2 months ago

Install screen on WP 5.1

Note: See TracTickets for help on using tickets.