Skip to content
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.

Sign up for GitHub

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

[ui] When Action output is long enough, keep a user scroll-anchored to the bottom #19452

Merged
merged 1 commit into from
Dec 13, 2023
Merged

[ui] When Action output is long enough, keep a user scroll-anchored to the bottom #19452

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/19452.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:improvement
ui: when an Action has long output, anchor to the latest messages
```
7 changes: 6 additions & 1 deletion ui/app/components/action-card.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,12 @@
<code><pre>Error: {{this.instance.error}}</pre></code>
{{/if}}
{{#if this.instance.messages.length}}
<code><pre>{{this.instance.messages}}</pre></code>
<code tabindex="0">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Side-effect: now you can focus in the action output and do things like pgdn etc.

<pre {{did-update this.anchorToBottom this.instance.messages}}>
{{this.instance.messages}}
</pre>
<div class="anchor" />
</code>
{{else}}
{{#if (eq this.instance.state "complete")}}
<p class="no-messages">Action completed with no output</p>
Expand Down
26 changes: 26 additions & 0 deletions ui/app/components/action-card.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,4 +48,30 @@ export default class ActionCardComponent extends Component {
// Either the passed instance, or the peer-selected instance
return this.selectedPeer || this.args.instance;
}

@tracked hasBeenAnchored = false;

/**
* Runs from the action-card template whenever instance.messages updates,
* and serves to keep the user's view anchored to the bottom of the messages.
* This uses a hidden element and the overflow-anchor css attribute, which
* keeps the element visible within the scrollable <code> block parent.
* A trick here is that, if the user scrolls up from the bottom of the block,
* we don't want to force them down to the bottom again on update, but we do
* want to keep them there by default (so they have the latest output).
* The hasBeenAnchored flag is used to track this state, and we do a little
* trick when the messages get long enough to cause a scroll to start the
* anchoring process here.
*
* @param {HTMLElement} element
*/
@action anchorToBottom(element) {
if (this.hasBeenAnchored) return;
const parentHeight = element.parentElement.clientHeight;
const elementHeight = element.clientHeight;
if (elementHeight > parentHeight) {
this.hasBeenAnchored = true;
element.parentElement.scroll(0, elementHeight);
}
}
}
20 changes: 18 additions & 2 deletions ui/app/styles/components/actions.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,14 +117,30 @@
}

.messages {
width: 100%;
overflow: hidden;

code > pre {
height: 200px;
code {
background-color: #0a0a0a;
color: whitesmoke;
display: block;
overflow: auto;
height: 200px;
border-radius: 6px;
resize: vertical;
pre {
background-color: transparent;
color: unset;
overflow-anchor: none;
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you have a chance to test on Safari, where overflow-anchor doesn't seem to be supported?
image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, this won't work in Safari; observed behaviour there is same as pre-PR (treats this like any other scrollable element, not "sticky")

min-height: 100%;
white-space: pre-wrap;
}
Comment on lines +131 to +137
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Part of this was making the scrollbars appear in the correct spot. An earlier attempt had me creating a scrollable area within the <pre> block, but that was unnecessary. Instead, we move the colo(u)r rules to the parent <code> element and have the <pre> always min-height to the code block's height.

.anchor {
overflow-anchor: auto;
height: 1px;
margin-top: -1px;
visibility: hidden;
}
}
}

Expand Down
Loading