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.

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

Web UI: Add a log viewer #4184

Conversation

M4rtinK
Copy link
Contributor

@M4rtinK M4rtinK commented Jun 9, 2022

Add a log viewer that opens in a modal window when the
"Show logs" button in the upper right corner of the screen
is clicked.

The log viewer is currently hardcoded to follow /tmp/anaconda.log.

@M4rtinK M4rtinK added f37 Fedora 37 webui labels Jun 9, 2022
@M4rtinK M4rtinK changed the title Add a log viewer Web UI: Add a log viewer Jun 9, 2022
@M4rtinK M4rtinK force-pushed the master-add_log_viewer_to_the_web_ui branch from cbbdd7c to 8aba366 Compare June 9, 2022 20:13
@M4rtinK
Copy link
Contributor Author

M4rtinK commented Jun 9, 2022

Snímek obrazovky 2022-06-09 v 22 12 14
This is basically a simplified version of the current log viewer mockup - it follows live updates to anaconda.log & supports search.
TODO (in separate PR):

  • log file selection
  • log levels
  • head/tail buttons
  • actual live version data
  • printscreen support

Remainig issues to fix for this PR:

  • move the modal somewhere else
  • unit test coverage
  • report PF bug on line numbers
  • ask PF team about modal sizing

@@ -179,6 +182,10 @@ const Footer = ({ isFormValid, setIsFormValid, setStepNotification, isInProgress
exitGui={exitGui}
setQuitWaitsConfirmation={setQuitWaitsConfirmation}
/>}
{showLogViewer &&
<LogViewerModal
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see a relation between a footer and a log viewer. There is no reason why the modal component should be here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Looks like for some reason we have many modals defined here - might be a good idea to move those as well to some more sensible place.

Copy link
Contributor

Choose a reason for hiding this comment

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

These modals are there because they are somehow tied to the actions of the footer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Turns out this indeed fits well on the top level next to the help drawer - just took me some time to figure it out. :P

ui/webui/src/components/LogViewer.jsx Outdated Show resolved Hide resolved
ui/webui/src/components/LogViewer.jsx Outdated Show resolved Hide resolved
ui/webui/src/components/LogViewer.jsx Outdated Show resolved Hide resolved
ui/webui/src/components/LogViewer.jsx Outdated Show resolved Hide resolved
@M4rtinK M4rtinK force-pushed the master-add_log_viewer_to_the_web_ui branch from 8aba366 to 1da6afc Compare June 10, 2022 15:57
@M4rtinK
Copy link
Contributor Author

M4rtinK commented Jun 10, 2022

PR updated - thanks a lot for all the feedback so far! :)
Snímek obrazovky 2022-06-10 v 17 56 06

@M4rtinK M4rtinK force-pushed the master-add_log_viewer_to_the_web_ui branch from 1da6afc to 0f7d76f Compare June 10, 2022 16:29
<Flex direction={{ default: "column" }}>
<TextContent>
<Text component={TextVariants.p}>
{_("Installer version") + " 37.10"}
Copy link
Contributor

Choose a reason for hiding this comment

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

The hardcoded version isn't very nice, but let's not drag webpack macros into this yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think eventually we want to just fetch it via DBus from the backend, right ? Or is it set somewhere else where we can reach, like the config file ?

Copy link
Contributor

Choose a reason for hiding this comment

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

For python we have pyanaconda/version.py.in, so I would expect the same...

ui/webui/src/components/LogViewerModal.jsx Outdated Show resolved Hide resolved
// - without this the line number column is too narrow
// and line numbers quickly become unreadable
.pf-c-log-viewer__index {
width: auto;
Copy link
Contributor

Choose a reason for hiding this comment

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

Doesn't this make the UI jump if new numbers are added and are longer? Eg. 99->100.

Perhaps also min-width: 3rem; or some such could be useful to prevent the log from jumping too much in the beginning when the render length of line numbers grows faster.

Copy link
Contributor

Choose a reason for hiding this comment

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

I wouldn't mess with it more that we do now. It is a workaround for a bug in Patternfly anyway.

@github-actions
Copy link

This PR is stale because it has been open 60 days with no activity.
Remove stale label or comment or this will be closed in 30 days.

@github-actions github-actions bot added the stale label Aug 15, 2022
@poncovka poncovka added f38 Fedora 38 and removed f37 Fedora 37 labels Aug 16, 2022
@github-actions github-actions bot removed the stale label Aug 17, 2022
@M4rtinK M4rtinK force-pushed the master-add_log_viewer_to_the_web_ui branch 2 times, most recently from 2038ec3 to 47bdc9a Compare September 14, 2022 01:24
@M4rtinK M4rtinK force-pushed the master-add_log_viewer_to_the_web_ui branch from 47bdc9a to e62d74c Compare September 26, 2022 17:15
Add a log viewer that opens in a modal window when the
"Show logs" button in the upper right corner of the screen
is clicked.

The log viewer is currently hardcoded to follow /tmp/anaconda.log.

Also add tests and resuable test functions.
@M4rtinK M4rtinK force-pushed the master-add_log_viewer_to_the_web_ui branch from e62d74c to da7c32c Compare September 27, 2022 00:59
Copy link
Contributor

@VladimirSlavik VladimirSlavik left a comment

Choose a reason for hiding this comment

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

I think this should be fine.

@github-actions
Copy link

This PR is stale because it has been open 60 days with no activity.
Remove stale label or comment or this will be closed in 30 days.

@github-actions github-actions bot added the stale label Dec 20, 2022
@github-actions
Copy link

This PR was closed because it has been stalled for 30 days with no activity.

@github-actions github-actions bot removed the stale label Jan 20, 2023
@github-actions
Copy link

This PR is stale because it has been open 60 days with no activity.
Remove stale label or comment or this will be closed in 30 days.

@github-actions github-actions bot added the stale label Mar 22, 2023
@github-actions
Copy link

This PR was closed because it has been stalled for 30 days with no activity.

@github-actions github-actions bot closed this Apr 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 participants