fix: console text selection#1834
Open
pedrolamas wants to merge 3 commits intofluidd-core:developfrom
Open
Conversation
Signed-off-by: Pedro Lamas <pedrolamas@gmail.com>
pedrolamas
added
the
Code - Optimizations
Contributor
There was a problem hiding this comment.
Pull request overview
This PR removes vue-virtual-scroller from the console implementation and replaces the virtualized DynamicScroller with a normal scrolling container/list to restore reliable mouse text selection in the console (and related update output consoles).
Changes:
- Replaced the console’s
DynamicScrollerwith a plain list inside a newAppScrollableContainerthat manages “pinned to latest” scrolling. - Removed
vue-virtual-scrollerdependency, plugin registration, CSS import, and related TypeScript typings. - Updated console item rendering to support both
ConsoleEntryandUpdateResponsepayload shapes.
Reviewed changes
Copilot reviewed 6 out of 9 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/typings/vue-virtual-scroller.d.ts | Removes local typings for vue-virtual-scroller. |
| src/main.ts | Removes vue-virtual-scroller CSS import and plugin registration. |
| src/components/widgets/console/ConsoleItem.vue | Broadens accepted item type and adjusts rendering guards for union types. |
| src/components/widgets/console/ConsoleCard.vue | Removes auto-scroll toggle UI and updates scroll-to-latest invocation. |
| src/components/widgets/console/Console.vue | Replaces DynamicScroller with AppScrollableContainer + v-for list rendering. |
| src/components/ui/AppScrollableContainer.vue | Adds a new reusable scroll container that tracks “away from latest” state. |
| package.json | Removes vue-virtual-scroller dependency. |
| package-lock.json | Removes vue-virtual-scroller and its transitive dependencies. |
| components.d.ts | Registers AppScrollableContainer for auto-imported components typing. |
Comment on lines
+40
to
+44
| updated () { | ||
| this.$nextTick(() => { | ||
| this.scrollToLatest() | ||
| }) | ||
| } |
Member
Author
There was a problem hiding this comment.
Acknowledged. This is a known trade-off of the simplified implementation. In practice the flipLayout option is rarely used, so the viewport drift impact is expected to be minimal. Can be revisited if it turns out to be a problem.
Signed-off-by: Pedro Lamas <pedrolamas@gmail.com>
Comment on lines
73
to
77
| </v-list-item-content> | ||
| </v-list-item> | ||
|
|
||
| <v-list-item @click="autoScroll = !autoScroll"> | ||
| <v-list-item-action class="my-0"> | ||
| <v-checkbox :input-value="autoScroll" /> | ||
| </v-list-item-action> | ||
| <v-list-item-content> | ||
| <v-list-item-title> | ||
| {{ $t('app.console.label.auto_scroll') }} | ||
| </v-list-item-title> | ||
| </v-list-item-content> | ||
| </v-list-item> | ||
|
|
||
| <v-list-item @click="flipLayout = !flipLayout"> | ||
| <v-list-item-action class="my-0"> |
Member
Author
There was a problem hiding this comment.
I want to keep the state property for now and will revisit this later.
Signed-off-by: Pedro Lamas <pedrolamas@gmail.com>
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.
Replaces and removes the DynamicScroller with a regular list. Main reason for this is to allow selection of text which the DynamicScroller control does not allow due to circular items recycling.
Fixes #1020
Fixes #1783
Replaces #1823