When you open the 'View differences' dialog (#1821548: Add a "diff" of some kind to the CMI UI) it jumps straight to the bottom (on OSX Chrome anyways.) On longer diffs this can be irritating. I believe that the reason this happens is because the selection point starts at the first link, which is on the bottom. However I'm not sure what, if anything, to do about it. Anyone?

CommentFileSizeAuthor
#9 diff_under_toolbar.png52.58 KBxjm
#6 1938514-3-review-before-patch.png61.16 KBjoates
#6 1938514-3-after.png46.01 KBjoates
#6 1821548-reduced-window.png41.26 KBjoates
#6 1821548-full-window.png48.75 KBjoates
#3 1938514-remove-back-link-from-diff-dialog-2.patch600 bytesgdd
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

YesCT’s picture

YesCT CreditAttribution: YesCT commented
Issue tags: +Needs issue summary update, +Novice, +CMI

(novice) issue summary initiative tag to use the template (issue summary template doc: http://drupal.org/node/1155816)
(novice) add steps to reproduce to issue summary (contributor task doc for writing steps to test: http://drupal.org/node/1468198)

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented
Issue tags: +Accessibility

This is because of a11y fixes from upstream Jquery ui for keyboard users, which we contributed some fixes for. Can we add a jump to bottom link at the top of the modal? This will get the focus and stop the scroll (I hope).

gdd’s picture

gdd
he/him
Primary language English
Location Portland, OR
CreditAttribution: gdd commented
Issue tags: -CMI +Configuration system
FileSize
1938514-remove-back-link-from-diff-dialog-2.patch600 bytes

I'd rather just get rid of the link entirely to be honest. You'll always have access to the close button regardless, and escape will close as well. Patch attached.

gdd’s picture

gdd
he/him
Primary language English
Location Portland, OR
CreditAttribution: gdd commented
Status: Active » Needs review
jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran commented
Issue tags: +modal dialog

Tagging.

joates’s picture

joates CreditAttribution: joates commented
Assigned: Unassigned » joates
Status: Needs review » Needs work
FileSize
1821548-full-window.png48.75 KB
1821548-reduced-window.png41.26 KB
1938514-3-after.png46.01 KB
1938514-3-review-before-patch.png61.16 KB
  1. created some config changes in my 8.x local staging area.
  2. needed to add the additional patch from #1843220-76: Convert form AJAX to new AJAX API to get the overlay re-working.

    3. screenshot (BEFORE the patch needing review)..
    1938514-3-review-before-patch.png

  3. applied the patch from #3

screenshot (AFTER applying the patch)..
1938514-3-after.png

Conclusion:

The links *is* removed but that change alone does not prevent the overlay from displaying incorrectly..

reducing the browser window height clearly shows that there is still a problem with initial dialog positioning :((
(the dialog title is chopped off, as is the close button !! -- the dialog can of course still be closed with escape key)
1821548-reduced-window.png

and finally with the full height browser window.. (this looks normal)
1821548-full-window.png

joates’s picture

joates CreditAttribution: joates commented
Assigned: joates » Unassigned

needs work.

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented

I believe the diff dialog also has bugged placement with the vertical-mode toolbar. When opened, it gets stuck underneath. See: #1847084: Measure/track displacing elements better + provide change events for them (fix overlay + toolbar). Not sure if these can be treated as separate bugs or not. :)

xjm’s picture

xjm
she/her
Primary language English
CreditAttribution: xjm commented
FileSize
diff_under_toolbar.png52.58 KB

Screenshot illustrating #8:
diff_under_toolbar.png

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented
Status: Needs work » Needs review
Issue tags: -Needs issue summary update, -Novice, -Accessibility, -modal dialog, -Configuration system

#3: 1938514-remove-back-link-from-diff-dialog-2.patch queued for re-testing.

Status: Needs review » Needs work
Issue tags: +Needs issue summary update, +Novice, +Accessibility, +modal dialog, +Configuration system

The last submitted patch, 1938514-remove-back-link-from-diff-dialog-2.patch, failed testing.

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented

What happened to config_admin_diff_page()?

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan commented

@mgifford - its moved to a controller, see /core/modules/config/lib/Drupal/config/Controller/ConfigController.php

mgifford’s picture

mgifford
he/him
Primary language English
CreditAttribution: mgifford commented

I wonder if $output['back'] = array has already been removed. It's not in ConfigController.php.

Might be this is no longer an issue. Will have to test this to see if we can close it.

nod_’s picture

nod_
Primary language French
Location Lille
CreditAttribution: nod_ commented
Status: Needs work » Closed (duplicate)

#2095225: Scrolling is broken on modals