Trying to address the fragment issue of points 4 and 6 from #1440628: [Meta] javascript toolbar/tableheader fragment mess | URI #hash.

Patch needs work, properly define/inlude js libraries, clean up some code, testing but basis should be there.

Files: 
CommentFileSizeAuthor
#10 core-js-offset-mess-1870006-10.patch2.77 KBnod_
PASSED: [[SimpleTest]]: [MySQL] 59,006 pass(es). View
#6 core-js-offset-mess-1870006-6.patch2.82 KBnod_
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch core-js-offset-mess-1870006-6.patch. Unable to apply patch. See the log in the details link for more information. View
#4 core-js-offset-mess-1870006-4.patch7.52 KBnod_
PASSED: [[SimpleTest]]: [MySQL] 50,766 pass(es). View
core-js-offset-mess.patch7.51 KBnod_
FAILED: [[SimpleTest]]: [MySQL] 49,382 pass(es), 1 fail(s), and 0 exception(s). View

Comments

nod_’s picture

Status: Active » Needs review

NR to get some opinions.

Status: Needs review » Needs work

The last submitted patch, core-js-offset-mess.patch, failed testing.

nod_’s picture

Status: Needs work » Needs review

Random faillure

nod_’s picture

FileSize
7.52 KB
PASSED: [[SimpleTest]]: [MySQL] 50,766 pass(es). View
nod_’s picture

Status: Needs review » Needs work

lots changed.

nod_’s picture

Status: Needs work » Needs review
FileSize
2.82 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch core-js-offset-mess-1870006-6.patch. Unable to apply patch. See the log in the details link for more information. View

Made a mess of everything, but it's upated to use the new Drupal.displace things and kind of works for fragments and html5 form errors. The sticky headers for tables are not taken into consideration but meh.

Putting NR for attention

casey’s picture

Priority: Normal » Major

#800270: Anchor/focus hidden beneath toolbar/tableheader (CKEditor full screen menu) also handled keyboard navigation (TAB). Unfortunately no time to review.

mgifford’s picture

Status: Needs review » Needs work
Issue tags: +JavaScript clean-up, +Needs JS testing

The last submitted patch, core-js-offset-mess-1870006-6.patch, failed testing.

nod_’s picture

Status: Needs work » Needs review
FileSize
2.77 KB
PASSED: [[SimpleTest]]: [MySQL] 59,006 pass(es). View
Wim Leers’s picture

Status: Needs review » Needs work
  1. +++ b/core/misc/offset/fragment.js
    @@ -0,0 +1,26 @@
    + * Offsets the window to allow fragments navigation with fixed positioning elements.
    

    80 col.

  2. +++ b/core/misc/offset/fragment.js
    @@ -0,0 +1,26 @@
    +  "use strict";
    ...
    +  function adjustOffset () {
    

    Why this indentation? We don't do that elsewhere either.

    Also in other JS file.

  3. +++ b/core/misc/offset/fragment.js
    @@ -0,0 +1,26 @@
    +  // Checks for a fragment on page load, outside Drupal behaviors.
    +  Drupal.behaviors.adjustOffset = { attach: adjustOffset };
    

    "outside" behaviors, yet you make it into a behavior?

  4. +++ b/core/misc/offset/html5forms.js
    @@ -0,0 +1,13 @@
    + * Focus the HTML5 validation error in view.
    

    Cool :) But where can I see this in action?

  5. +++ b/core/modules/system/system.module
    @@ -952,6 +952,10 @@ function system_library_info() {
           'core/misc/drupal.js' => array('group' => JS_LIBRARY, 'weight' => -18),
    +      'core/misc/debounce.js' => array('group' => JS_LIBRARY),
    +      'core/misc/displace.js' => array('group' => JS_LIBRARY),
    +      'core/misc/offset/fragment.js' => array('group' => JS_LIBRARY),
    +      'core/misc/offset/html5forms.js' => array('group' => JS_LIBRARY),
    

    Why? Because they can be considered fundamental?

    Shouldn't the html5forms thing be included with the drupal.form library, and drupal.displace only be loaded when the Toolbar is active, i.e. a dependency of Toolbar's library?

nod_’s picture

That's too cosmetic for the state the patch is in :þ

To test the HTML5 validation, just create a node, don't put a title, scroll down and try to save and publish.

jessebeach’s picture

+++ b/core/modules/system/system.module
@@ -952,6 +952,10 @@ function system_library_info() {
       'core/misc/drupal.js' => array('group' => JS_LIBRARY, 'weight' => -18),
+      'core/misc/debounce.js' => array('group' => JS_LIBRARY),
+      'core/misc/displace.js' => array('group' => JS_LIBRARY),
+      'core/misc/offset/fragment.js' => array('group' => JS_LIBRARY),
+      'core/misc/offset/html5forms.js' => array('group' => JS_LIBRARY),

I agree, I'm having trouble parsing this. So it makes no sense for drupal.js to depend on the displace.js library, since displace.js itself relies on drupal.js. But since the displace library depends on the drupal library, we end up in something of a loop.

The only resolution I can come up with is explicitly declaring dependencies on the debounce, displace, and fragment libraries and letting them all depend on the drupal library, since it declares the Drupal object.

nod_’s picture

I put that in to not think about all this.

It's really the architecture of the JS code that needs work here before we get into anything else.

klonos’s picture

mgifford’s picture

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

mgifford’s picture

Issue tags: +accessibility

I'm tagging this for accessibility as it's required to get inline form errors as a fully fledged part of Core.

dmsmidt’s picture

dmsmidt’s picture

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.9 was released on September 7 and is the final bugfix release for the Drupal 8.1.x series. Drupal 8.1.x will not receive any further development aside from security fixes. Drupal 8.2.0-rc1 is now available and sites should prepare to upgrade to 8.2.0.

Bug reports should be targeted against the 8.2.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.3.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.