WordPress.org

Make WordPress Core

Opened 4 years ago

Closed 4 years ago

#13045 closed enhancement (duplicate)

Login: Add Classes for better plugin-support and change from <p> to <div> for more flexibility

Reported by: F J Kaiser Owned by:
Milestone: Priority: normal
Severity: minor Version: 2.9.2
Component: Administration Keywords:
Focuses: Cc:

Description

(This ticket relates to: http://core.trac.wordpress.org/ticket/12506)

+ <p> have more limits than <div>s or <span>s. I changed the messages above and the links below the form.
+ Eliminated the admin-color-scheme (see related ticket above).
+ Added Class .login_link for links below form, so plugin-authors don't have to make new classes, but can override them.
+ Added Class .user_data for form-fields for plugin-authors.

I'm not the best programmer, so this needs someone who takes a look if the provided patch is really working or just in my test-environment.

Goal of both patches is a) making wp-login.php an independent module, b) bring more flexibility and c) let plugin-authors scale with more re-useable code.

Attachments (5)

wp-login.php (22.3 KB) - added by F J Kaiser 4 years ago.
13045.diff (18.6 KB) - added by markmcwilliams 4 years ago.
Based on wp-login.php attachment, a patch showing the changes!
wp-login.2.php (25.3 KB) - added by F J Kaiser 4 years ago.
from 3.0-nightly build (download 50 min ago)
13045.2.diff (5.9 KB) - added by nacin 4 years ago.
From FJK.
wp-login.patch (3.5 KB) - added by F J Kaiser 4 years ago.
minor additions; get's rid of admin-color-css-file

Download all attachments as: .zip

Change History (17)

F J Kaiser4 years ago

comment:1 ocean904 years ago

Hey, can you use the wp-login.php from the trunk and add only a patch with the differences? Thanks.

comment:2 F J Kaiser4 years ago

  • Cc 24-7@… added

markmcwilliams4 years ago

Based on wp-login.php attachment, a patch showing the changes!

comment:3 markmcwilliams4 years ago

  • Keywords dev-feedback 2nd-opinion added; wp-login.php login html needs 2nd opinion needs testing removed

So based on the patch submitted, I created a patch showing all the changes that were made, I have not got round to testing it, but that's the patch showing all the differences! :P Although looking through the changes, I see quite a bit has been altered, and the 'shake it' part has been removed, I like that part! - I'm just helping!

comment:4 follow-up: F J Kaiser4 years ago

@markmcwilliams: Thanks for your help with this. I still can't get around the login to svn (pass or username not working).

I changed the wp-login.php from my original 2.9.2-file, and as far as i understood, you made the changes to the 3.0-file (that's because there's so much missing now). My changes were just
a) changing all <p>-messages to <div>-messages
b) removing the hook with the admincolor-css-file

I'll try to get my fingers on a 3.0-file and upload a new version (or someone can help me with my login, so i can offer a diff-patch. Thanks!).

comment:5 nacin4 years ago

  • Milestone changed from 3.0 to Unassigned

I tried looking through the patch and picked out what this ticket tried to achieve, and from what I saw, I'm -1 on the changes.

Most of these classes, if not all, are being added to elements that already have IDs and attributes that can be targeted via CSS.

Additionally, there is no desire to convert to divs, versus paragraphs, as they have zero semantic value. We've had that debate previously on the comment form, for example. When dealing with styling, paragraphs and divs are both block elements, so the only restrictions are when you start talking about child block elements (which there aren't).

We now have wp_login_form() which ideally could even be used in wp-login.php. It's already a very independent module, and I'm not really sure what the color scheme has to do with it. Any changes to styles in one would need to happen in the other. That said, I don't think there is *any* reason to change elements or even add classes.

Also, you should only be svn co http://core.svn.wordpress.org/trunk . Username/pass would be for commits.

comment:6 in reply to: ↑ 4 markmcwilliams4 years ago

Replying to F J Kaiser:

I changed the wp-login.php from my original 2.9.2-file, and as far as i understood, you made the changes to the 3.0-file (that's because there's so much missing now).

Ahh, that'd make sense I guess!

comment:7 F J Kaiser4 years ago

Hi nacin and markmcwilliams and thanks for the help & feedback. To make it easier to take a look at the changes, i added comments (search for "change") in the next "patch" (file taken from 3.0-nightly build). It's just to give you a faster & easier overview about the changes (i really can't login to svn. It works with wp.org and the plugin-rep, but not for core - sry 'bout this, i'll find out another day).

In general i just try to unbind the admin-color-scheme from the login-page as described in the other ticket.

In detail i just changed a) stop calling admin-color-scheme b) login messages. But nothing inside the <form>, so wp_login_form() won't be affected (no need for changes) by login.css or wp-login.php. When you're talking about semantic value then the provided patch should do it. If you think it's not, then i'd suggest to not house the error-messages in a paragraph, but an <ul>. (And yes, i was talking about child-block-elements added by f/ex a plugin-author or later wp-versions).

My only *real* goal is to get rid of the admin-color-scheme and the id="login_error" (should be class in my humble opinion), because it makes things a little bit hard for plugin-authors or people who try to style the login in their theme. The other changes are just additions i thought that could be useful.

F J Kaiser4 years ago

from 3.0-nightly build (download 50 min ago)

nacin4 years ago

From FJK.

comment:8 follow-up: nacin4 years ago

It's just to give you a faster & easier overview about the changes (i really can't login to svn. It works with wp.org and the plugin-rep, but not for core - sry 'bout this, i'll find out another day).

You don't log in. Simply check out the repository.

I diff'ed the changes, though I really don't see a point in the changes. Not to mention that it would break all existing styling by plugins.

comment:9 in reply to: ↑ 8 F J Kaiser4 years ago

<Replying to nacin:

You don't log in. Simply check out the repository.

My Problem is only when i tried to commit the files, i get a 403 with my login (i use TortoiseSVN). Am i allowed to commit, or only to provide diff-patches (didn't read that anywhere)?

<Replying to nacin:

I diff'ed the changes, though I really don't see a point in the changes. Not to mention that it would break all existing styling by plugins.

As you might have seen: I'm no pro at svn, but i was able to make a diff too (proud of myself ;) ). Please take a look: I took out all changes at the html of the messages i made previously. What i left in is only an addition: a) imput.user-data, because the IDs are not in use as IDs and plugins or later versions may benefit from a basic styling-class too and b) .login_error as an addition to the ID in case someone want's to make his own error-div or the messages will get both "paragraphed" or "dived" in a later version (still don't understand why there's a difference).

F J Kaiser4 years ago

minor additions; get's rid of admin-color-css-file

comment:10 follow-up: nacin4 years ago

No, only the core developers have commit access. Commit means to actually change WordPress code.

I don't see why we need to add classes when these elements already have IDs, classes, or both. The color scheme discussion should be handled in #12506, not here.

comment:11 in reply to: ↑ 10 F J Kaiser4 years ago

Replying to nacin:

No, only the core developers have commit access. Commit means to actually change WordPress code.

Ok. Thanks for the info.
Replying to nacin:

I don't see why we need to add classes when these elements already have IDs, classes, or both. The color scheme discussion should be handled in #12506, not here.

Ok. Originally it was just meant to make the other patch complete. I think we can say, that we ain't got the same oppinion on the div/p-tags and therefore just leave it out. Let's end the discussion over here and leave the ticket open until someone from the core-dev team cares about it. I hope it's a sollution with which we can both live :)

comment:12 nacin4 years ago

  • Keywords dev-feedback 2nd-opinion removed
  • Milestone Unassigned deleted
  • Resolution set to duplicate
  • Status changed from new to closed

closing then. #12506

Note: See TracTickets for help on using tickets.