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

[#1401] improvement(dashboard): Fixed distorted page display due to excessively long property values #2009

Merged
merged 2 commits into from
Aug 3, 2024

Conversation

yl09099
Copy link
Contributor

@yl09099 yl09099 commented Aug 2, 2024

What changes were proposed in this pull request?

Fixed the property value is too long and the page display is distorted.

  1. If the content of the cell is too long, use ellipses to replace the excess.
  2. Provide bubbles to show everything.

image
image

Why are the changes needed?

Fix: #1401

Does this PR introduce any user-facing change?

Yes.

How was this patch tested?

Page display.

@yl09099
Copy link
Contributor Author

yl09099 commented Aug 2, 2024

@rickyma @maobaolong @xianjingfeng PATL.

Copy link

github-actions bot commented Aug 2, 2024

Test Results

 2 747 files  ±0   2 747 suites  ±0   5h 38m 33s ⏱️ -50s
   979 tests ±0     978 ✅ ±0   1 💤 ±0  0 ❌ ±0 
12 294 runs  ±0  12 279 ✅ ±0  15 💤 ±0  0 ❌ ±0 

Results for commit ac86d67. ± Comparison against base commit 77e8612.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@advancedxy advancedxy left a comment

Choose a reason for hiding this comment

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

Others lgtm.

@@ -127,7 +127,7 @@ Mock.mock(/\/coordinator\/conf/, 'get', {
},
{
argumentKey: 'java.home',
argumentValue: '/home/hadoop/jdk-11.0.2'
argumentValue: '/root/jdk-11.0.2'
Copy link
Contributor

Choose a reason for hiding this comment

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

Why these changes? It's automatically generated in your environment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There was a mistake in the substitution.fixed.

@maobaolong
Copy link
Member

@yl09099 After #1951 this PR merged, it should not have unrelated System properties, do you get your screenshot after #1951 this PR?

@yl09099
Copy link
Contributor Author

yl09099 commented Aug 2, 2024

@yl09099 After #1951 this PR merged, it should not have unrelated System properties, do you get your screenshot after #1951 this PR?

Understood, I may use the original version here, it is OK this PR does not affect, sometimes the path is too long will not look good, use this will not affect the beauty.

@maobaolong
Copy link
Member

can I search a text within a long value after this PR?

@yl09099
Copy link
Contributor Author

yl09099 commented Aug 3, 2024

can I search a text within a long value after this PR?

This was a very good suggestion and I decided to add a search window, which is done.

Copy link
Member

@maobaolong maobaolong left a comment

Choose a reason for hiding this comment

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

@yl09099 This PR looks pretty good, thanks for your contribution!

@rickyma rickyma changed the title [#1401] improvement(dashboard): Fixed the property value is too long and the page display is distorted. [#1401] improvement(dashboard): Fixed distorted page display due to excessively long property values Aug 3, 2024
@rickyma rickyma merged commit 41389df into apache:master Aug 3, 2024
43 checks passed
@rickyma
Copy link
Contributor

rickyma commented Aug 3, 2024

Merged. Thanks @yl09099 @advancedxy @maobaolong

maobaolong pushed a commit to maobaolong/incubator-uniffle that referenced this pull request Aug 6, 2024
…e to excessively long property values (apache#2009)

### What changes were proposed in this pull request?

Fixed the property value is too long and the page display is distorted.
1. If the content of the cell is too long, use ellipses to replace the excess.
2. Provide bubbles to show everything.

![image](https://github.com/user-attachments/assets/83d4f962-03b7-4c15-82fd-070e7e875c8a)
![image](https://github.com/user-attachments/assets/a9c4add7-e461-4332-bbba-c8ca9c9d5740)

### Why are the changes needed?

Fix: apache#1401

### Does this PR introduce _any_ user-facing change?

Yes.

### How was this patch tested?

Page display.

(cherry picked from commit 41389df)
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.

[Umbrella] Enhance dashboard capabilities.
4 participants