-
Notifications
You must be signed in to change notification settings - Fork 4.7k
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
[Improvement-16947][UI] Task instance log details should always stay at the bottom #16947
base: dev
Are you sure you want to change the base?
Conversation
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.
Please follow the pull request notice first.
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.
I think this feature needs disscussion. No all users needs this when the log is very large and refresh very fast.
Well, should I use it like this? [Improvement-16947][UI] Task instance log details should always stay at the bottom |
Add a button that allows the user to manually control whether to scroll to the bottom, without forcing automatic scrolling. Would that work? |
You can take a look at https://github.com/apache/dolphinscheduler/blob/dev/docs/docs/en/contribute/join/pull-request.md |
I think it's a good idea. |
The code has been submitted |
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.
Generally LGTM. Just some NIT.
You didn't create a new improvement issue and link to this PR. @imizao |
Co-authored-by: xiangzihao <[email protected]>
Co-authored-by: xiangzihao <[email protected]>
Fixed the NITs, thanks for the suggestions! |
@@ -67,8 +71,12 @@ export default defineComponent({ | |||
const { t } = useI18n() | |||
|
|||
const variables = reactive({ | |||
isFullscreen: false | |||
isFullscreen: false, | |||
autoScrollToBottom: true // New state variable |
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.
If we set to autoScrollToBottom
to true by default. We should increase batch query log line num to 100000. This can significantly reduce the number of requests and improve the user experience.
dolphinscheduler/dolphinscheduler-ui/src/views/projects/task/instance/batch-task.tsx
Line 128 in 352b47b
variables.limit += 1000 |
dolphinscheduler/dolphinscheduler-ui/src/views/projects/task/instance/stream-task.tsx
Line 115 in 352b47b
variables.limit = 1000 |
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.
Thanks for the suggestion! In normal cases, a limit of 1000 is sufficient for most scenarios. The GIF I provided was an exceptional example to demonstrate the auto-scroll-to-bottom functionality, but it doesn't reflect the typical usage. Therefore, I believe keeping the limit at 1000 is appropriate for now. If we encounter performance issues or higher demand in the future, we can revisit this setting.
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.
In the production environment, 1000 is not enough for automatic refresh. It will cause great performance pressure on both frontend and backend. And we should avoid repeated execution and set a delay for it.
Quality Gate passedIssues Measures |
The |
Purpose of the pull request
Task instance log details should always stay at the bottom.
close #16975
Brief change log
The log details area for task instances should always remain scrolled to the bottom, allowing users to view the latest log output in real-time.
Final effect 1
Final effect 2
Verify this pull request
This pull request is code cleanup without any test coverage.
Pull Request Notice
Pull Request Notice
If your pull request contains incompatible change, you should also add it to
docs/docs/en/guide/upgrade/incompatible.md