Replace jquery.layout and jQuery UI with a flexbox splitter - #24
Merged
Merged
Conversation
jQuery UI was 250K carried for two jobs, neither of them a widget the console
used: making one Bootstrap dialog draggable and resizable, and satisfying
jquery.layout, which calls .draggable() unguarded for its pane resizers.
jquery.layout itself was abandoned in 2013, was the last vendored file we
shipped, and had already needed hand-patching for jQuery 4 earlier today.
App.Util.Splitter replaces it: one flexible pane, one sized pane and a bar
between them, on flexbox and pointer events. setPointerCapture keeps the drag
alive when the pointer crosses the editor, which is the cross-browser problem
jQuery UI existed to solve and which the platform now handles directly.
content-view keeps its nested arrangement — west against the rest, then editor
against results — and its orientation switching, size persistence and collapse,
now expressed as three splitters rather than two jquery.layout instances.
Bootstrap has no splitter, draggable or resizable, so it could not have replaced
either library; the modals were already Bootstrap and stay that way. Their drag
and resize move into modal.coffee as pointer handlers, about sixty lines.
Four bugs found and fixed while verifying, all in the new code:
- Both inner splitters share .outer-center, so setting the flex direction in the
constructor let whichever was built last pin the container to its own axis and
orientation switching did nothing. Direction is applied in show().
- setPointerCapture was unguarded. Where it throws, the handler died before
binding pointermove and the bar was inert. Now tolerated.
- Stored pane sizes are percentages ('50%' by default, as jquery.layout
accepted), which arithmetic turned into NaN. Sizes now resolve against the
container.
- show() returned early when a pane was already visible, so the sizing pass
after attach never ran and panes were unsized on first paint. It now always
re-applies, with onAttach driving the post-attach pass.
The resizer also goes from 3px to 6px; the original was a fiddly target.
Payload falls from 300K in 4 files to 248K in 3 — no vendor directory at all
now, just the bundle, its stylesheet and the favicon — and 250K of jQuery UI
stops being fetched.
Verified in the bundled app with settings cleared: panes open at the 50% default
(east 521 of a 1042 container), both axes drag and persist, orientation switches
between side-by-side and stacked, results hide and restore, the scripts panel
opens and resizes, sizes survive a reload, the Save-as dialog drags and resizes
by exactly the pointer delta, a script executes, jQuery.ui and jQuery.fn.layout
are both undefined, and nothing fails to load. 33 jasmine specs pass.
jquery.layout highlighted its resizer on hover and gave it a "Resize" tooltip. The flexbox splitter had neither, so the only cue that the bar could be dragged was the cursor change once already over it — and with no resting colour of its own it was invisible against the panes. The bar now carries its resting and hover colours itself rather than depending on the chrome-scoped rule, which was not winning in practice and left the bar transparent. The chrome blocks still override for dark. Dragging keeps its stronger blue, so the three states read as resting, available and active. It also gains title="Resize", role="separator" and aria-orientation. jquery .layout's resizer was an unlabelled div; a screen reader can now at least name it, though keyboard resizing remains unimplemented.
The separator was labelled but inert: a keyboard user could reach nothing and resize nothing, which is no better than the unlabelled div jquery.layout left behind. The bar is now focusable and driven by the keys the ARIA authoring practices specify for a separator. Arrow keys move it along its own axis in 12px steps, shift for 60, and Home and End jump to the minimum and maximum. Movement is expressed as right/down positive and then mapped through @before exactly as a drag is, so a pane grows in the direction the bar visually travels and the two input paths cannot drift apart. Arrows across the other axis are left alone rather than swallowed, so they still scroll the page. A focusable separator is the one ARIA permits to carry value attributes, so aria-valuenow, -valuemin and -valuemax are maintained alongside the size and a screen reader can announce the position as it changes. :focus-visible gives the bar the same blue it takes while dragging, plus an outline. Held keys would otherwise write to localStorage on every repeat, so persistence is debounced to one write 250ms after the last keypress; pointer drags still persist immediately on release. Verified in the bundled app on both axes. Vertical: Home pins the pane to the 40px minimum, ArrowUp steps 40 to 52 to 64, shift-ArrowUp jumps to 124, ArrowDown returns to 112, aria-valuenow tracks each change and 112 is persisted once after the debounce. Horizontal: ArrowLeft grows the pane 480 to 492, shift-ArrowLeft to 552, ArrowRight back to 540, End reaches the maximum. Arrows on the inactive axis change nothing. 33 jasmine specs pass.
The polyfill restored $.fn.selector, removed in jQuery 3, and its own header
says why it was here: "Required for older plugins like jQuery Layout that rely
on this property". jquery.layout is gone, and nothing else reads the property —
Marionette's remaining .selector references are definition.selector on a region
definition object, its own option rather than jQuery's, and this app declares
regions as plain strings anyway.
That leaves jquery.hotkeys as the only vendored file the plugin ships;
jasmine-jquery is test-only and already excluded from packaging.
Verified in the bundled app: $('body').selector is undefined, Marionette's
regions still resolve, the five splitter panes build, CodeMirror 6 loads, a
global Ctrl-Enter still executes through jquery.hotkeys, and nothing fails to
load. 33 jasmine specs pass.
CodeMirror 5 ended a handled keystroke with e_stop, which is preventDefault and stopPropagation together, so it never reached the document-level bindings in _initKeybindings. CodeMirror 6's keymap only calls preventDefault, so since the editor moved to it every Mod-Enter inside the editor executed the script twice and every Mod-s saved twice. The document bindings exist so the shortcuts work with focus anywhere — the results prompt, the scripts panel, nothing at all — so they stay, and instead skip events that originated inside .cm-editor, where the editor's own keymap already owns them. Verified in the bundled app: Mod-Enter with focus in the editor now produces exactly one result where it previously produced two, and Mod-Enter with focus on the body still executes, so the global path is intact. 33 jasmine specs pass.
The plugin matched on event.keyCode, deprecated in the UI Events spec, and had been unmaintained since 2010. It was also the last vendored file the console shipped; web/vendor now holds only jasmine-jquery, which is test-only and already excluded from packaging, so nothing vendored reaches the jar. App.Util.Keys carries the two guards rather than scattering them. isTyping reproduces the filter jquery.hotkeys applied to indirect bindings, without which Esc would start clearing the output while you type a filename. isFromEditor keeps the document-level shortcuts off keys the editor's own keymap has already handled, since CodeMirror 6 calls preventDefault but not stopPropagation. Handlers bound straight to the results prompt skip both, exactly as jquery.hotkeys exempted direct bindings. Two combos needed care rather than a literal translation. 'Ctrl+s Meta+s' becomes a case-insensitive match, because the key is 'S' when shift is held. 'Shift+return return' fires for Enter with or without shift but not with a command modifier, so the prompt keeps handing shift-Enter to onExecute — which inserts a newline rather than running — and keeps ignoring Mod-Enter, which belongs to the global binding. Verified in the bundled app: global Mod-Enter from the body executes; Mod-Enter in the editor executes exactly once; Mod-Enter and Escape while focus is in a textarea do nothing; Enter in the results prompt executes and clears it; Shift-Enter does not execute and leaves the text; ArrowUp recalls the previous entry and ArrowDown returns to empty. jQuery.hotkeys is undefined. 33 jasmine specs pass.
Five of the eleven pinned versions were redundant. grails-bom manages bootstrap and bootstrap-icons, exactly as it manages jQuery — all three at the versions that were written out here by hand. Only jQuery had been left to the platform, so the other two are now declared without a version too, their gradle.properties entries are gone, and the generated links resolve them from the classpath at render time with no build-time fallback. That also empties the last use of gradle.properties in the gulp build, so paths.js no longer parses it. Of the six CodeMirror artifacts, @codemirror/state, view and language are dependencies of the other three, so declaring commands, legacy-modes and theme-one-dark is enough to bring the whole graph. Their pins are removed as well. Eleven pinned versions become three, and only for artifacts nothing else depends on. Verified: the runtime classpath still resolves all sixteen webjars at the same versions as before — state 6.7.1, view 6.43.6, language 6.12.4 now transitively — every URL the console page references returns 200, CodeMirror loads with groovy highlighting, jQuery reports 4.0.0, Bootstrap is present, the five splitter panes build and a script executes. 33 jasmine specs pass.
Collaborator
|
What goes on here? Merging without reviews? |
1 similar comment
Collaborator
|
What goes on here? Merging without reviews? |
Contributor
Author
|
@sbglasius non apache project. 8.0.x is my branch. I created it. Every commit on it is mine,. The entire branch needs to be reviewed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
jQuery UI was 250K carried for two jobs, neither of them a widget the console used:
.draggable()unguarded for its pane resizers.jquery.layout was abandoned in 2013, was the last vendored file we shipped, and had already needed hand-patching for jQuery 4 in #21.
Bootstrap could not have replaced either — it has no splitter, no draggable and no resizable. The modals were already Bootstrap and stay that way.
What replaces them
App.Util.Splitter: one flexible pane, one sized pane, a bar between them, built on flexbox and pointer events.setPointerCapturekeeps the drag alive when the pointer crosses the editor — the cross-browser problem jQuery UI existed to solve, which the platform now handles directly.content-viewkeeps its nested arrangement (west against the rest, then editor against results) plus orientation switching, size persistence and collapse — now three splitters instead of two jquery.layout instances. The modal's drag and resize become pointer handlers inmodal.coffee, about sixty lines.Bugs found while verifying, all in the new code
.outer-center, so setting the flex direction in the constructor let whichever was built last pin the container to its own axis — orientation switching did nothing. Direction is applied inshow().setPointerCapturewas unguarded. Where it throws, the handler died before bindingpointermoveand the bar was inert.'50%'by default, as jquery.layout accepted), which arithmetic turned intoNaN. Sizes now resolve against the container.show()returned early when a pane was already visible, so the sizing pass after attach never ran and panes were unsized on first paint. It now always re-applies, withonAttachdriving the post-attach pass.The resizer also goes from 3px to 6px; the original was a fiddly target.
Result
Payload falls from 300K in 4 files to 248K in 3 — no
vendor/directory at all now, just the bundle, its stylesheet and the favicon — and 250K of jQuery UI stops being fetched.Verification
In the bundled app with settings cleared: panes open at the 50% default (east 521 of a 1042 container); both axes drag and persist; orientation switches between side-by-side and stacked; results hide and restore; the scripts panel opens and resizes; sizes survive a reload; the Save-as dialog drags and resizes by exactly the pointer delta; a script executes;
jQuery.uiandjQuery.fn.layoutare bothundefined; nothing fails to load. 33 jasmine specs pass.