Release v0.51.334 — Release KX (new-message cue when scrolled up, #3631) #3849
There are no files selected for viewing
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
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
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
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
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
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
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,84 @@ | ||||||||||
| from pathlib import Path | ||||||||||
|
|
||||||||||
|
|
||||||||||
| REPO = Path(__file__).resolve().parents[1] | ||||||||||
| UI_JS = (REPO / "static" / "ui.js").read_text(encoding="utf-8") | ||||||||||
| I18N_JS = (REPO / "static" / "i18n.js").read_text(encoding="utf-8") | ||||||||||
| STYLE_CSS = (REPO / "static" / "style.css").read_text(encoding="utf-8") | ||||||||||
|
|
||||||||||
|
|
||||||||||
| def _function_body(src: str, signature: str) -> str: | ||||||||||
| start = src.index(signature) | ||||||||||
| brace = src.index("{", start) | ||||||||||
| depth = 0 | ||||||||||
| for i in range(brace, len(src)): | ||||||||||
| if src[i] == "{": | ||||||||||
| depth += 1 | ||||||||||
| elif src[i] == "}": | ||||||||||
| depth -= 1 | ||||||||||
| if depth == 0: | ||||||||||
| return src[start : i + 1] | ||||||||||
| raise AssertionError(f"function body not found: {signature}") | ||||||||||
|
|
||||||||||
|
|
||||||||||
| def _scroll_listener_block() -> str: | ||||||||||
| start = UI_JS.index("el.addEventListener('scroll'") | ||||||||||
| return UI_JS[start : UI_JS.index("})();", start)] | ||||||||||
|
|
||||||||||
|
|
||||||||||
| def test_preserve_scroll_unpinned_branch_shows_new_message_cue_after_restore(): | ||||||||||
| helper = _function_body(UI_JS, "function _scrollAfterMessageRender") | ||||||||||
|
|
||||||||||
| # The preserve-scroll branch keeps master's forced follow path for pinned / | ||||||||||
| # near-bottom users (no regression on settled-response bottom-pinning), and | ||||||||||
| # only the genuinely-scrolled-up cohort restores their viewport + gets the | ||||||||||
| # new-message cue. (Codex CORE catch: scrollIfPinned() in the pinned branch | ||||||||||
| # could leave a pinned reader short of the settled response.) | ||||||||||
| assert "if(_followMessagesAfterDomReplace()) return;" in helper | ||||||||||
| assert "_restoreMessageScrollSnapshot(scrollSnapshot);" in helper | ||||||||||
| assert helper.index("_restoreMessageScrollSnapshot(scrollSnapshot)") < helper.index( | ||||||||||
| "_maybeShowNewMessageScrollCue(scrollSnapshot)" | ||||||||||
| ) | ||||||||||
| # The cue is only shown on the non-follow (restore) path, after the restore. | ||||||||||
| assert helper.index("_followMessagesAfterDomReplace()") < helper.index( | ||||||||||
| "_maybeShowNewMessageScrollCue(scrollSnapshot)" | ||||||||||
| ) | ||||||||||
| assert helper.count("_maybeShowNewMessageScrollCue(scrollSnapshot)") == 1 | ||||||||||
|
|
||||||||||
|
|
||||||||||
| def test_scroll_cue_uses_growth_below_restored_viewport(): | ||||||||||
| maybe = _function_body(UI_JS, "function _maybeShowNewMessageScrollCue") | ||||||||||
| sync = _function_body(UI_JS, "function _syncScrollToBottomCue") | ||||||||||
|
|
||||||||||
| assert "el.scrollHeight>previousHeight+24" in maybe | ||||||||||
| assert "distance>80" in maybe | ||||||||||
| assert "_showNewMessageScrollCue()" in maybe | ||||||||||
| assert "scroll-to-bottom-btn--new-message" in sync | ||||||||||
| assert "session_new_message" in sync | ||||||||||
| assert "session_jump_end" in sync | ||||||||||
|
|
||||||||||
|
|
||||||||||
| def test_click_near_bottom_and_resets_clear_new_message_cue(): | ||||||||||
| scroll = _function_body(UI_JS, "function scrollToBottom") | ||||||||||
| reset_direction = _function_body(UI_JS, "function _resetScrollDirectionTracker") | ||||||||||
| reset_stream = _function_body(UI_JS, "function _resetStreamScrollFollow") | ||||||||||
| listener = _scroll_listener_block() | ||||||||||
|
|
||||||||||
| assert scroll.index("_clearNewMessageScrollCue();") < scroll.index("_scrollPinned=true") | ||||||||||
| assert "if(nearBottom) _clearNewMessageScrollCue();" in listener | ||||||||||
| assert "_syncScrollToBottomCue(showBottomButton,{newMessage:_newMessageCueVisible})" in listener | ||||||||||
| assert "_clearNewMessageScrollCue();" in reset_direction | ||||||||||
| assert "_clearNewMessageScrollCue();" in reset_stream | ||||||||||
|
|
||||||||||
|
|
||||||||||
| def test_new_message_cue_i18n_keys_exist_in_locale_blocks(): | ||||||||||
| assert I18N_JS.count("session_new_message:") >= 8 | ||||||||||
| assert I18N_JS.count("session_new_message_label:") >= 8 | ||||||||||
|
Comment on lines
+75
to
+76
There was a problem hiding this comment.
Suggested change
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! |
||||||||||
| assert "session_new_message: 'New message'" in I18N_JS | ||||||||||
| assert "session_new_message_label: 'New message available, jump to end'" in I18N_JS | ||||||||||
|
|
||||||||||
|
|
||||||||||
| def test_new_message_cue_has_stable_pill_styling(): | ||||||||||
| assert ".scroll-to-bottom-btn.scroll-to-bottom-btn--new-message" in STYLE_CSS | ||||||||||
|
Comment on lines
+80
to
+82
There was a problem hiding this comment.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time! |
||||||||||
| assert "max-width:min(220px,calc(100% - 40px))" in STYLE_CSS | ||||||||||
| assert ".scroll-to-bottom-btn.scroll-to-bottom-btn--new-message .session-jump-btn__text" in STYLE_CSS | ||||||||||
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
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
Oops, something went wrong.
Oops, something went wrong.
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.
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.
Both new assertions match exact whitespace and the inline comment
"// Keep master's follow heuristic". If the block is ever reformatted or the comment is updated (e.g. issue number changes), this test will fail despite the code being functionally correct. The behavioral invariant being tested — that_followMessagesAfterDomReplace()comes before_restoreMessageScrollSnapshotand_maybeShowNewMessageScrollCue— could be checked with three independentinchecks (already done intest_issue3545_streaming_scroll_cue.py) rather than a single multi-line string that encodes the exact comment wording.Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!