-
-
Notifications
You must be signed in to change notification settings - Fork 18
BL-15657 replace ElementQueries to fix origami bug #7592
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: Version6.3
Are you sure you want to change the base?
Conversation
JohnThomson
left a comment
There was a problem hiding this 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.
nabalone
left a comment
There was a problem hiding this 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
|
Should this target 6.3? |
nabalone
left a comment
There was a problem hiding this 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.
This change is