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

V8: Apply nicer log message wrapping #5274

Merged
merged 1 commit into from May 31, 2019

Conversation

kjac
Copy link
Contributor

@kjac kjac commented Apr 16, 2019

Prerequisites

  • I have added steps to test this contribution in the description below

If there's an existing issue for this PR then this fixes #5230

Description

See #5230

Currently the log viewer messages break out of the table if the window is too small to contain them - like this:

log-viewer-before

With this PR applied, the messages stay inside the table:

log-viewer-after

@bjarnef
Copy link
Contributor

bjarnef commented Apr 16, 2019

Alternatively the table could have some horizontal scroll similar to this https://codepen.io/springborg/pen/MvPmPP
Then long log messages don't force the table row to grow.

@kjac
Copy link
Contributor Author

kjac commented Apr 16, 2019

@bjarnef I really don't want to introduce JS just to scroll the table 😄

Jokes aside, I did try with a horizontal scrollbar (basically a container with overflow-x: auto;). But it really doesn't work well, since the scrollbar is in the bottom of the very long list of log results:

log-viewer-with-overflow

@bjarnef
Copy link
Contributor

bjarnef commented Apr 16, 2019

@kjac you don't have to, e.g. when having the table wrapped inside a div something like in Bootstrap https://getbootstrap.com/docs/3.3/css/#tables-responsive

But I see the issue when having many rows and just a bit scroll and scrollbar is at bottom. Maybe the horisontal scroll and paging could be fixed at bottom in some way, but not sure if it is possible without adding some javascript.

@kjac
Copy link
Contributor Author

kjac commented Apr 16, 2019

@bjarnef the example above is done in CSS only 👍

I sincerely doubt anything can be done about the scrollbar at the bottom without a bunch of magic JS. You're welcome to give it a go if you fancy.

@poornimanayar
Copy link
Contributor

Hi @kjac ,

I have tested this as is and it looks okay. The only thing I could think of is that if you have any exception which has huge amounts of text you will have to begin scrolling quite endlessly. and this happens on a normal size screen as well.

image

image

Poornima

@kjac
Copy link
Contributor Author

kjac commented May 2, 2019

@poornimanayar yeah this is what Bjarne and I were discussing earlier too. A horizontal scrollbar could be added, but it would end up in the bottom of all the log messages, meaning you would have to scroll to the bottom of the list in order to scroll horizontally. Probably not the best user experience either...?

@poornimanayar
Copy link
Contributor

yes @kjac , thats why i left it there. Its one of those changes :-)

Poornima

@nul800sebastiaan nul800sebastiaan merged commit ebc21a1 into umbraco:v8/dev May 31, 2019
@nul800sebastiaan
Copy link
Member

I think this is the best solution for now, it could turn into a bit of scrolling indeed but a horizontal scroll is pretty unusable. Thanks @kjac! 👍

@kjac kjac deleted the v8-fix-logviewer-wordwrap branch May 31, 2019 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

v8: Log message should wrap inside container
4 participants