Skip to content

BL-15657 replace ElementQueries to fix origami bug#7592

Merged
JohnThomson merged 1 commit intoVersion6.3from
BL-15657_origami_fix_replacing_elementQueries
Jan 26, 2026
Merged

BL-15657 replace ElementQueries to fix origami bug#7592
JohnThomson merged 1 commit intoVersion6.3from
BL-15657_origami_fix_replacing_elementQueries

Conversation

@nabalone
Copy link
Copy Markdown
Contributor

@nabalone nabalone commented Jan 17, 2026

This change is Reviewable


Open with Devin

Copy link
Copy Markdown
Contributor

@JohnThomson JohnThomson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JohnThomson reviewed 5 files and all commit messages, and made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @nabalone).


a discussion (no related file):
It's elegant that you've replaced an obsolete library with modern CSS, but I can't spot what actually fixes the bug.

Copy link
Copy Markdown
Contributor Author

@nabalone nabalone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nabalone made 1 comment.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @JohnThomson).


a discussion (no related file):

Previously, JohnThomson (John Thomson) wrote…

It's elegant that you've replaced an obsolete library with modern CSS, but I can't spot what actually fixes the bug.

We had been relying on ElementQueries to add min-height and min-width attributes to the split-pane-container-inner, which would then make the css rules apply. The bug occurred because ElementQueries was not adding those attributes in certain specific cases, I didn't investigate why. But now with the container queries, the css rules should apply any time the split-pane-container-inner is the specified size

@andrew-polk
Copy link
Copy Markdown
Contributor

Should this target 6.3?

@nabalone nabalone changed the base branch from master to Version6.3 January 21, 2026 15:37
Copy link
Copy Markdown
Contributor Author

@nabalone nabalone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops yes fixed

@nabalone made 2 comments.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @JohnThomson).


a discussion (no related file):

Previously, nabalone (Noel) wrote…

We had been relying on ElementQueries to add min-height and min-width attributes to the split-pane-container-inner, which would then make the css rules apply. The bug occurred because ElementQueries was not adding those attributes in certain specific cases, I didn't investigate why. But now with the container queries, the css rules should apply any time the split-pane-container-inner is the specified size

Oh, I see, I forgot to explain the problem! The (+) buttons for creating a new split are positioned with css which assumes that vertical-adders has a reasonable size and then we can do I think it was position: absolute; top: 10px or something like that to position them. But when the aforementioned css rules were unexpectedly not applying, the result was that vertical-adders got height:0, so then what was supposed to be the top (+) button with position: absolute; top: 10px ended up on bottom and the bottom (+) button ended up on top and so they would create unexpected origami splits.

Copy link
Copy Markdown
Contributor

@JohnThomson JohnThomson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JohnThomson resolved 1 discussion.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @nabalone).

@JohnThomson JohnThomson merged commit 116008e into Version6.3 Jan 26, 2026
2 checks passed
@JohnThomson JohnThomson deleted the BL-15657_origami_fix_replacing_elementQueries branch January 26, 2026 17:46
Copy link
Copy Markdown

@devin-ai-integration devin-ai-integration Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

✅ Devin Review: No Issues Found

Devin Review analyzed this PR and found no potential bugs to report.

View in Devin Review to see 4 additional findings.

Open in Devin Review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants