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

Add overflow-x scroll #43552

Merged
merged 1 commit into from
Sep 30, 2024
Merged

Add overflow-x scroll #43552

merged 1 commit into from
Sep 30, 2024

Conversation

mcruzdev
Copy link
Contributor

@mcruzdev mcruzdev commented Sep 27, 2024

Changes

Apply overflow-x: scroll and display: block to .stacktrace class.

Old behavior

image

New behavior

Now the container is not overflowing:

Screenshot 2024-09-27 at 00 25 53

Fixes #43551

@mcruzdev mcruzdev marked this pull request as ready for review September 27, 2024 03:27
Copy link
Member

@gsmet gsmet left a comment

Choose a reason for hiding this comment

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

Should we wrap instead? It sounds like a better behavior than having to scroll?

@phillip-kruger
Copy link
Member

I have no preference for wrap or scroll :) Either way happy with the change.

Copy link

quarkus-bot bot commented Sep 30, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit c9ea0c6.

Failing Jobs

Status Name Step Failures Logs Raw logs Build scan
Native Tests - Data6 Upload build reports ⚠️ Check → Logs Raw logs 🔍

You can consult the Develocity build scans.


Flaky tests - Develocity

⚙️ JVM Tests - JDK 21

📦 extensions/smallrye-reactive-messaging/deployment

io.quarkus.smallrye.reactivemessaging.hotreload.ConnectorChangeTest.testUpdatingConnector - History

  • Expecting actual: ["-4","-5","-6","-7","-8","-9","-10","-11"] to start with: ["-3", "-4", "-5", "-6"] - java.lang.AssertionError
java.lang.AssertionError: 

Expecting actual:
  ["-4","-5","-6","-7","-8","-9","-10","-11"]
to start with:
  ["-3", "-4", "-5", "-6"]

	at io.quarkus.smallrye.reactivemessaging.hotreload.ConnectorChangeTest.testUpdatingConnector(ConnectorChangeTest.java:36)

⚙️ Native Tests - Security2

📦 integration-tests/oidc-client-wiremock

io.quarkus.it.keycloak.OidcClientInGraalITCase.testGetAccessTokenWithConfiguredExpiresIn - History

  • expected: <true> but was: <false> - org.opentest4j.AssertionFailedError
org.opentest4j.AssertionFailedError: expected: <true> but was: <false>
	at io.quarkus.it.keycloak.OidcClientTest.testGetAccessTokenWithConfiguredExpiresIn(OidcClientTest.java:60)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)
	at io.quarkus.test.junit.QuarkusTestExtension.interceptTestMethod(QuarkusTestExtension.java:810)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

@geoand geoand merged commit 4665414 into quarkusio:main Sep 30, 2024
51 of 52 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.16 - main milestone Sep 30, 2024
@gsmet
Copy link
Member

gsmet commented Sep 30, 2024

Sorry, I should have been a bit more assertive in my review as I think we really should switch to wrapping. While this PR is an improvement over the existing behavior, having to scroll is really annoying, either when trying to see the whole line or even when copy/pasting the error.

We have a good example of that when pasting stack trace in GitHub with backticks, it can quickly get annoying and sometimes, I even switch to quoting so that you have the nice wrapping behavior.

@mcruzdev any chance you could create a follow-up PR with some wrapping instead as you seem to agree with it in your reaction? Thanks!

@mcruzdev
Copy link
Contributor Author

Sorry, I should have been a bit more assertive in my review as I think we really should switch to wrapping. While this PR is an improvement over the existing behavior, having to scroll is really annoying, either when trying to see the whole line or even when copy/pasting the error.

We have a good example of that when pasting stack trace in GitHub with backticks, it can quickly get annoying and sometimes, I even switch to quoting so that you have the nice wrapping behavior.

@mcruzdev any chance you could create a follow-up PR with some wrapping instead as you seem to agree with it in your reaction? Thanks!

Hi @gsmet obviously! Another point that I would to talk about is TemplateHtmlBuilder, we Java code with HTML code together what is the possibility to change it to use Qute?

This question above is for the future (maybe), I will create another PR that adds wrapping behavior, thank you!

@mcruzdev mcruzdev deleted the issue-43551 branch September 30, 2024 11:42
@gsmet
Copy link
Member

gsmet commented Sep 30, 2024

I think the point of using some low level code instead of Qute is to avoid having a dependency on Qute in virtually every app as the Dev UI or error pages would require it.

It's not ideal but sometimes you have to stay a bit more low level.

@mcruzdev
Copy link
Contributor Author

Hmmm... It makes sense! TY

@phillip-kruger
Copy link
Member

Maybe we can add two small icons. One that copies the stackstrace into clipboard and one that switch between wrapped and non-wrapped (default to wrapped ?) w.d.y.t ?

@gsmet
Copy link
Member

gsmet commented Sep 30, 2024

Do you see a reason why you would prefer it unwrapped? I must admit I can't see a good reason for it.

@phillip-kruger
Copy link
Member

I just think some people might prefer that ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Stacktrace container overflows in error page
4 participants