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

Display multi-line messages correctly in query results #2935

Merged
merged 3 commits into from Oct 17, 2018

Conversation

Projects
None yet
2 participants
@MattIrv
Member

MattIrv commented Oct 17, 2018

This modifies how we render query messages to support multi-line messages. Previously new lines were stripped from messages, meaning that things like stack traces did not display correctly, and each message was hardcoded to 22px height since it was only ever a single line. Now the height of each message is calculated dynamically based on the number of lines in the message.

This also fixes a couple other bugs with how we displayed messages:

  • Maintains the message scroll position when switching between query tabs, which used to work but broke recently
  • If a query returned so many messages that it scrolled the message window the message window would start scrolled all the way down instead of starting at the top

Before:
Messages window showing multiple errors including stack traces. Each message is displayed on a single line and the user would have to scroll very far to the right to see the stack traces

After:
Messages window showing multiple errors including stack traces with proper line breaks in the stack traces and other messages

@MattIrv MattIrv requested a review from anthonydresser Oct 17, 2018

let lines = resultMessage.message.split('\n').length;
this.messageLineCountMap.set(resultMessage, lines);
this.totalLines += lines;
this.maximumBodySize = this.totalLines * 22;

This comment has been minimized.

@anthonydresser

anthonydresser Oct 17, 2018

Member

Does it make more sense to change the maximum body size after doing all the counts rather than after each?

This comment has been minimized.

@MattIrv

MattIrv Oct 17, 2018

Member

Good idea, just fixed it. Does everything else look good?

@@ -145,27 +147,37 @@ export class MessagePanel extends ViewletPanel {
let hasError = false;
if (isArray(message)) {
hasError = message.find(e => e.isError) ? true : false;
message.forEach(resultMessage => this.countMessageLines(resultMessage));

This comment has been minimized.

@anthonydresser

anthonydresser Oct 17, 2018

Member

small thing, this could be changed to a reduce method and remove the property from the object. Then use that reduce result to resize the maximum body size.

This comment has been minimized.

@MattIrv

MattIrv Oct 17, 2018

Member

good catch, done

@MattIrv MattIrv merged commit 5a0490e into master Oct 17, 2018

1 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
license/cla All CLA requirements met.
Details

@MattIrv MattIrv deleted the fix/singleLineMessages branch Oct 17, 2018

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