Keep the close X usable on columns with hidden titles - #6
Open
zattak1 wants to merge 1 commit into
Open
Conversation
ddf3149 replaced the JS height calculation with flexbox and, to stop the column title from taking vertical space when Q_columns_hideTitle is set, hid the title outright: .Communities .Q_columns_column.Q_columns_hideTitle .Q_columns_title { display: none; } That also removes the close X. columns.css puts .Q_close inside the title and, for hidden titles, deliberately keeps it visible while hiding only the inner container: .Q_columns_hideTitle .Q_close { opacity: 1; } .Q_columns_hideTitle .Q_columns_title_container { display: none; } With the title display:none the X is never rendered, so a column opened with a hidden title (event detail, for example) has no close affordance -- getBoundingClientRect() on the title returns 0x0. Keep the title rendered and take it out of the flex flow instead: a 50x50 absolutely-positioned overlay in the column's top-right corner. Columns are already positioned elements (columns.js sets position relative/absolute on every column), so the overlay anchors to its own column. pointer-events: none keeps it from intercepting clicks meant for the column content, and .Q_close opts back in. The height fix is unaffected -- the title no longer occupies vertical space or pushes content down. Verified on a two-column desktop layout: the overlay lands at the detail column's top-right, elementFromPoint over the X returns .Q_close, and elementFromPoint over the column body returns the content tool.
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
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.
The problem
A column with
Q_columns_hideTitlehas no close X, so once it is open there is no way to close it.ddf3149("Replace JavaScript height calculations with CSS Flexbox") stops the title from taking vertical space on such columns by hiding it outright —web/css/Communities.css:But
Q/web/css/tools/columns.cssputs.Q_closeinside the title, and for hidden titles it deliberately keeps the X visible while hiding only the inner container:display: noneon the title takes the X down with it. Theopacity: 1rule above is left with nothing to act on.Repro
Q_columns_hideTitle(the event detail column on a Calendars-backed page is the one we hit).document.querySelector('.Q_columns_hideTitle .Q_columns_title').getBoundingClientRect()returns0 x 0.Present on
maintoday; introduced byddf3149.The fix
Keep the title rendered, but take it out of the column's flex flow and shrink it to a 50×50 overlay in the top-right corner. The height fix is preserved — the title still occupies no vertical space and pushes nothing down — and the X comes back.
pointer-events: noneon the overlay keeps it from swallowing clicks meant for the column content;.Q_closeopts back in.No JS changes, and nothing outside the
Q_columns_hideTitlecase is touched.Why the overlay anchors correctly
Q/web/js/tools/columns.jssetspositionon every column as it opens —absoluteon mobile,relativeotherwise:So each column is already a containing block and the overlay resolves against its own column, not against the columns tool. No extra
positionrule is needed.Verification
Checked on a live two-column desktop layout (
Q_notMobile, event detail open asQ_column_1):offsetParentis the column itself.elementFromPointat the X's centre returns.Q_close.elementFromPointover the column body returns the content tool, confirming the overlay does not intercept content clicks.opacity: 1, ascolumns.cssintends.Unrelated, noted in passing
On our install the X's
<img>stays at the lazy-load placeholder (Q/img/throbbers/transparent.gif) withdata-lazyload-srcpointing atQ/img/x.png, so even with this fix the glyph can come up blank depending on lazy-load behaviour. That is a separate issue in the lazy-load path rather than anything to do with these rules, and this PR does not touch it.