frontend: Add toggle wrap for logViewer and fix log viewer styles by vyncent-t · Pull Request #3124 · kubernetes-sigs/headlamp

vyncent-t · 2025-04-15T20:16:29Z

This PR adds a toggle button for the Logs Viewer window that allows the text to be formatted for word wrap. There are also style formatting issues that I have noticed and made an issue for here #3052 this PR also adds changes that style error while also.

Changes

Introduces new Wrap option for log viewer component to allow toggle between default text formatting and word wrap
Introduces new format and tooltip context for log viewer buttons
- At current, adding additional switches or buttons to the log viewer components for both pod and non pod log viewer introduces styling errors (this PR makes it so that we can continue to add more features by reworking the button layout)
Fixes rendering for small view
- At current, when viewing log viewer on smaller windows the buttons are both misaligned and not styled correctly Log viewer buttons are not aligned #3052

How to test

Open headlamp into a cluster
Navigate to pods
Click on any pod
Open the "View logs" button
Click the toggle "Wrap" button
Log text format should change

Fork PR from original: #3027
requested feature from #2657

k8s-ci-robot · 2025-04-15T20:16:37Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: vyncent-t
Once this PR has been reviewed and has the lgtm label, please assign sniok for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

joaquimrocha

Besides my comments, I noticed that when we wrap the text, the vertical scroll bar is now pushed beyond the limits of the dialog, meaning we lose the ability to understand where we are vertically.

frontend/src/components/common/LogViewer.tsx

joaquimrocha · 2025-04-23T14:37:18Z

frontend/src/components/common/LogViewer.tsx

+}
+
+// // this function manually formats the logs to a single line.
+// // currently we must manually format the texts as addonFit does not work with multiline texts.


Can you elaborate on this? I don't understand exactly why this function is needed. Are you sure there's no way to delegate this to the xterm component?

I have tried using options for single line printing but it had a few issues.

the format to single line is needed since the log "lines" are not actually single line text strings but rather a bundle of log lines, which I found the terminal has trouble formatting it correctly. It also dynamically sets the width depending on the longest singular line, which that value I use to fix the width sizing.

(while the split new line \n is read properly by the console log, in the xterm it was not)
logs w/no timestamp

side note: I was responding in the context of you meaning "delegate" to the xterm comp as in using the built in options to handle the single line format printing (changing width, using addonFit, etc.) but if you mean in terms of moving this function from a stand alone into the main log viewer component I moved it there as well.

e2e-tests/tests/headlampPage.ts

vyncent-t · 2025-04-23T16:35:01Z

e2e tests are passing, none of the log changes cause it to fail. going to drop e2e test changes here and rebase when #3144 is merged

sniok · 2025-04-25T12:18:40Z

frontend/src/components/common/LogViewer.test.ts

+
+  it('returns 0 for empty input', () => {
+    expect(findLongestLine([])).toBe(0);
+  });


please add a test case like ["abcde", "egf\nabc"], I think the current implementation doesn't handle a case like that

I think 'splits on real LF (\n) for a line' covers it since the new version of the findLongestLine uses .split(/\n|\n|\r/) so should split as needed but I added an another one with ["abcde", "egf\nabc"]

sniok · 2025-04-28T09:20:16Z

frontend/src/components/common/LogViewer.test.ts

+  });
+
+  it('splits on real LF (\\n) for a line - longer', () => {
+    expect(findLongestLine(['abcde', 'egf\nabc'])).toBe(5 + 1);


the longest line in this case is abcdeefg and is 7 characters long

there isn't any joining after the function finds a new line or return syntax, it treats every "main string" (bundle of logs in live) as a collection that needs to be split up into parts and substrings and we count off those smaller parts

the inputs of ['abcde', 'egf\nabc'] are broken into ['abcde', 'egf' abc']

we then add a +1 on every line for the longest to make up for splitting the \n (it also makes sure the width is set to exactly the number of chars +1 to ensure a fit every time)

so the function should read
abcde as 6 long
efg as 4 long
abc as 4 long

yes I understand that currently the function works this way, but what I'm saying is that the function needs to be adjusted so it passes this test since it something we might encounter and it should properly count the line length

okay I think I see what you're saying, you mean that in the event the logs are chopped up like this

["this is a long line that for this current example that","for some reason\n we split"]

we should have it so that it doesn't break visually. I was confused as I only seen related logs paired together.

vyncent-t · 2025-05-29T17:37:33Z

blocked here due to new issues:

scroll issues, default Y scroll works on some pod logs and not others, some pod logs scrolling also no longer accurate? (was working before, works as intended but only when wrap lines is toggled off?)
cannot format the dead space while also having both scroll bars visible

vyncent-t · 2025-06-04T19:04:34Z

new issues

drawer mode: background does not flex fit when clicking one log, turning off wrap mode, then clicking another pod log to view it

vyncent-t · 2025-06-04T19:05:32Z

blocked here due to new issues:

scroll issues, default Y scroll works on some pod logs and not others, some pod logs scrolling also no longer accurate? (was working before, works as intended but only when wrap lines is toggled off?)
cannot format the dead space while also having both scroll bars visible

new issues

drawer mode: background does not flex fit when clicking one log, turning off wrap mode, then clicking another pod log to view it

vyncent-t · 2025-06-10T14:19:57Z

Wrap toggle is inverted, it wraps lines when the toggle is off

All the toggles can should be placed here on wide screens

fixed toggle inverse, since the logs currently load in in wrapped format I have fixed it to start as "Wrap lines" enabled

WIP: fixing the extra space

Need feedback: All toggles and drop downs on one line in wide view

I have addressed a few issues in the current format where all toggles and drop downs are all on the same line in this issue Log viewer buttons are not aligned #3052 . The changes I have currently not only fix the rendering issues, but also make things more future proof for adding more inputs later. I noticed that pods and deployments log viewer are different so by separating the drop downs above the toggles we can keep the look uniform.

Pod view logs

Other view logs

Notice that by putting them all on one line here, we start to collide into the side icon buttons, preventing us from adding another button on the same line in the future.

made some changes and I am now using grid for the space filling, will keep the non pod log viewer the same style of stacking since it does not have the space for it

ref: just adding as a visual ref for

vyncent-t · 2025-06-10T19:14:33Z

last push:

moved to mix of grid and box for new display

vyncent-t · 2025-06-27T18:09:19Z

May need to settle with the bottom spacing imperfections when not using wrap texts (toggling wrap text off) for some log windows.

This is only an issue for logs that do not contain enough lines to fill out the rest of the spacing when reformatting to unwrap.

I have also tried experimenting with creating a custom scroll bar just for this view and that introduces too many issues for what it is trying to accomplish (having to sync it to the 'real' scroll bar, having format issues with other switches due to needing nested boxes, etc.) and will be abandoning that idea.

Although I have been able to succeed in making the unwrap view work along with the Prettify option where it preserves the JSON readable format but with unwrapped lines.

…g viewer style - Adds a toggle switch in the log viewer that allows for texts to be wrapped on/off - Fixes issue where log viewer buttons were not aligned correctly Signed-off-by: Vincent T <vtaylor@microsoft.com>

…and add wrap toggle switch

sniok · 2025-07-11T14:57:14Z

xterm seems to be way limited, I think we should consider some alternative instead, that's more geared towards displaying logs instead of terminal emulator

k8s-ci-robot · 2025-07-17T01:12:11Z

PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 15, 2025

vyncent-t self-assigned this Apr 15, 2025

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 15, 2025

k8s-ci-robot requested review from illume and sniok April 15, 2025 20:16

k8s-ci-robot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Apr 15, 2025

vyncent-t force-pushed the log-viewer-wrap-toggle-button branch from 6eaabb4 to a8566bc Compare April 18, 2025 17:26

vyncent-t changed the title ~~[NEW] frontend: Add toggle wrap for logViewer and fix log viewer styles~~ frontend: Add toggle wrap for logViewer and fix log viewer styles Apr 18, 2025

vyncent-t force-pushed the log-viewer-wrap-toggle-button branch from a8566bc to 9adfd3e Compare April 22, 2025 19:08

vyncent-t marked this pull request as ready for review April 22, 2025 19:50

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Apr 22, 2025

k8s-ci-robot requested review from ashu8912 and joaquimrocha April 22, 2025 19:50

vyncent-t mentioned this pull request Apr 22, 2025

Fix e2e auth logic + add namespace a11y playwright tests #3144

Merged

joaquimrocha requested changes Apr 23, 2025

View reviewed changes

vyncent-t force-pushed the log-viewer-wrap-toggle-button branch 4 times, most recently from 0a54b04 to 2996431 Compare April 23, 2025 16:12

vyncent-t force-pushed the log-viewer-wrap-toggle-button branch 5 times, most recently from 563db9c to 07ffb46 Compare April 24, 2025 18:55

sniok reviewed Apr 25, 2025

View reviewed changes

vyncent-t force-pushed the log-viewer-wrap-toggle-button branch from 07ffb46 to 2e3adbf Compare April 25, 2025 14:33

sniok reviewed Apr 28, 2025

View reviewed changes

vyncent-t force-pushed the log-viewer-wrap-toggle-button branch from 2e3adbf to f98572a Compare April 29, 2025 14:42

k8s-ci-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels May 27, 2025

vyncent-t force-pushed the log-viewer-wrap-toggle-button branch 3 times, most recently from e84a9e3 to f40b78c Compare May 28, 2025 19:26

vyncent-t marked this pull request as draft May 28, 2025 19:27

k8s-ci-robot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label May 28, 2025

vyncent-t force-pushed the log-viewer-wrap-toggle-button branch from c1d21a1 to febba81 Compare June 10, 2025 13:09

vyncent-t force-pushed the log-viewer-wrap-toggle-button branch from 7a68143 to d0bbf9b Compare June 10, 2025 14:47

joaquimrocha added this to Headlamp Release Plan / Roadmap Jun 23, 2025

github-project-automation bot moved this to Queued in Headlamp Release Plan / Roadmap Jun 23, 2025

joaquimrocha moved this from Queued to Done in Headlamp Release Plan / Roadmap Jun 24, 2025

vyncent-t force-pushed the log-viewer-wrap-toggle-button branch 3 times, most recently from 91fb417 to 82e9374 Compare June 27, 2025 17:55

vyncent-t marked this pull request as ready for review June 27, 2025 18:09

k8s-ci-robot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jun 27, 2025

vyncent-t added 2 commits July 8, 2025 08:34

frontend: PodLogViewer Details.tsx: Rework log viewer button styling …

ad1d907

…and add wrap toggle switch

vyncent-t force-pushed the log-viewer-wrap-toggle-button branch from 82e9374 to ad1d907 Compare July 8, 2025 12:34

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 17, 2025

vyncent-t closed this Aug 25, 2025

Conversation

vyncent-t commented Apr 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

How to test

Uh oh!

k8s-ci-robot commented Apr 15, 2025

Uh oh!

joaquimrocha left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

vyncent-t commented Apr 23, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vyncent-t commented May 29, 2025

Uh oh!

vyncent-t commented Jun 4, 2025

Uh oh!

vyncent-t commented Jun 4, 2025

Uh oh!

vyncent-t commented Jun 10, 2025

Uh oh!

vyncent-t commented Jun 10, 2025

Uh oh!

vyncent-t commented Jun 27, 2025

Uh oh!

sniok commented Jul 11, 2025

Uh oh!

k8s-ci-robot commented Jul 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

vyncent-t commented Apr 15, 2025 •

edited

Loading