Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a helper for the getting of the login submit element #631

Merged
merged 1 commit into from
Jan 5, 2023

Conversation

eiriksm
Copy link
Contributor

@eiriksm eiriksm commented Nov 22, 2022

OK, so I have a use case it's kind of convoluted to achieve at the moment.

The login method will fill in fields and submit the form to log in. If for some reason you have changed the default markup, you can override the way this is selected by providing a value for the "drupal text" log_in.

This is convenient if you use English, and only that. That way you could probably add "Log in" and be done with it. If you want to accommodate that the site can theoretically at different stages have this part translated, or not, then using the string for the value of the submit is not so convenient. Luckily, we use the method findButton which does this:

    /**
     * Finds button (input[type=submit|image|button|reset], button) with specified locator.
     *
     * @param string $locator button id, value or alt

OK, so I can use the ID or the value. Since value is out of the question, let's try ID. So I change it to "#edit-submit", which is the ID of the submit button.

Well, guess what. Drupal can change that based on the order of cache being saved. So theoretically, you can have that as the id of the button at one point, but then something re-renders, you visit a page with a search form, then visit the login form while the search form is there. Now the search form has that as its id on the same page. Inconvenient.

But OK, let's just override how we submit it. I would like to do that. What I now would have to do is override the login method of the authentication manager, and copy paste all the code in there. What if I could just override one method of the manager instead, and use a different selector. For example, I could swap it out to just use

$submit = $element->find('css', '.user-login-form [data-drupal-selector="edit-submit"]');

That would be possible if we added a protected method for getting the element. A much safer and maintainable override.

I would also mention that it would be more convenient to have it as a "drupal selector", so if that is more interesting, I could create another PR for that?

@jhedstrom
Copy link
Owner

I like this idea, but I wonder if we are substituting one issue for another? This method relies on the form having that specific class, which in theory is theme-dependent. Could we possible use this to fallback in case the text method currently in place fails?

@eiriksm
Copy link
Contributor Author

eiriksm commented Dec 23, 2022

Very valid point! I'll see if I can get that in there

@eiriksm
Copy link
Contributor Author

eiriksm commented Dec 23, 2022

Wait, I think you are misunderstanding.

The PR contains the exact same code as earlier, it just makes it possible for me to override it on a per-project basis. I feel that's a good first step? So this PR is totally backwards compatible and works exactly like the current version of this package. Feels safest to me?

@jhedstrom
Copy link
Owner

The PR contains the exact same code as earlier, it just makes it possible for me to override it on a per-project basis.

Ah, yes, I did misread the change. This looks good. Thanks!

@jhedstrom jhedstrom merged commit 5deb56c into jhedstrom:master Jan 5, 2023
@eiriksm eiriksm deleted the feat/helper-for-submit branch January 5, 2023 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants