Problem/Motivation

In Drupal.AjaxCommands.prototype.webformScrollTop we look for a form element with this code:
var $form = $(response.selector + '-content').find('form');

But it turns out the form never exists inside that wrapper, because the wrapper is opened and closed in the #prefix, rather than opened in the #prefix and closed in the #suffix:

$form += ['#prefix' => '', '#suffix' => ''];
$form['#prefix'] .= '<span id="' . $wrapper_id . '-content"></span>';
$form['#prefix'] .= '<div' . $wrapper_attributes . '>';
$form['#suffix'] = '</div>' . $form['#suffix'];

This means that $form.hasClass('js-webform-autofocus') will always return false because $form is an empty jquery object.

Steps to reproduce

Proposed resolution

Close the wrapping span in the suffix:

$form += ['#prefix' => '', '#suffix' => ''];
$form['#prefix'] .= '<span id="' . $wrapper_id . '-content">';
$form['#prefix'] .= '<div' . $wrapper_attributes . '>';
$form['#suffix'] = '</div></span>' . $form['#suffix'];

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork webform-3452360

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

mstrelan created an issue. See original summary.

Maithri Shetty made their first commit to this issue’s fork.

maithri shetty’s picture

maithri shetty commented
Assigned: Unassigned » maithri shetty

Maithri Shetty opened merge request !475

maithri shetty’s picture

maithri shetty commented
Assigned: maithri shetty » Unassigned
Status: Active » Needs review

@mstrelan I have raised a PR for requested change. Can you please provide me the steps to reproduce the issue so that I can cross verify in my local as well. Thank you.

mstrelan’s picture

mstrelan commented
  1. Install Drupal standard profile
  2. Install Webform and Webform UI module
  3. Edit the default Contact webform and enabled AJAX
  4. Go to /form/contact and hit submit to trigger a validation error
  5. Inspect the page markup and notice an empty span above the webform, rather than wrapping the webform
maithri shetty’s picture

maithri shetty commented
Issue tags: +@mstrelan cross verified and works fine. Thank you
maithri shetty’s picture

maithri shetty commented
Issue tags: -@mstrelan cross verified and works fine. Thank you

@mstrelan cross verified and works fine. Thank you

mh_nichts’s picture

mh_nichts
she/her
Primary language French
commented

Hello,

Thanks for filing this issue.

However, I suggest solving it in another way, as using a span to wrap a div and a whole form is invalid according to W3C HTML standards.

My suggestion would be :

  • in JS : remove '-content' from the selector, so it targets the initial wrapper div which effectively contains the form
  • in PHP : keep span where it is, but add tabindex="-1" so it can effectively get the focus (currently it can't, whether wrapping the form or not)

I will try to commit these changes next week.

mh_nichts opened merge request !496

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented
Version: 6.2.x-dev » 6.3.x-dev

Please rebase against 6.3.x.

mh_nichts’s picture

mh_nichts
she/her
Primary language French
commented
Version: 6.3.x-dev » 6.2.x-dev

@lkmorlan Thank you ! It should be OK now, I could apply the patch from the MR locally.

mh_nichts’s picture

mh_nichts
she/her
Primary language French
commented
Version: 6.2.x-dev » 6.3.x-dev

jrockowitz changed the visibility of the branch 3452360-form-wrapper-focus-scroll to hidden.

jrockowitz’s picture

jrockowitz commented
Component: Code » JavaScript / Ajax

We can't have a DIV wrapper with a SPAN but we can convert the SPAN to a DIV and then wrap the form,

mh_nichts’s picture

mh_nichts
she/her
Primary language French
commented

@jrockowitz Thanks for the changes.
It seems OK for me regarding the structure, but it still misses tabindex="-1" to effectively get the focus (as a div is not focusable by default).

jrockowitz’s picture

jrockowitz commented

Done!

jrockowitz’s picture

jrockowitz commented
Status: Needs review » Reviewed & tested by the community

liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented
Status: Reviewed & tested by the community » Fixed

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.