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

[Dashboard] Actor Table UI Optimize #26785

Merged
merged 21 commits into from
Jul 25, 2022

Conversation

SongGuyang
Copy link
Contributor

Why are these changes needed?

Copy from #16071.
The related PR #15802.

Copy link
Contributor

@alanwguo alanwguo left a comment

Choose a reason for hiding this comment

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

Thanks! mostly lgtm. One small change.

import copy from "copy-to-clipboard";
import React, { useState } from "react";

const CopyableCollapse: React.FC<{
Copy link
Contributor

Choose a reason for hiding this comment

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

seems like this could be handled by pure css instead.

display: block
text-overflow: ellipsis
text-wrap: nowrap
overflow: hidden
width: 50px

You can see an example here (see idCol css class):

<Link to={`/node/${raylet.nodeId}`} className={classes.idCol}>

This css rule will cut off the text after 50px but if a user selects it and copies it, it will copy the entire string.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Make sense!

@SongGuyang
Copy link
Contributor Author

@alanwguo Take a look again or approve?

@@ -210,17 +194,28 @@ const ActorTable = ({
}
key={actorId}
>
<TableCell align="center">
{/* <div className={classes.idCol}>{actorId}</div> */}
Copy link
Contributor

Choose a reason for hiding this comment

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

left this comment in by accident?

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 forgot to delete, thx.

@SongGuyang
Copy link
Contributor Author

serve:test_kv_store is flakey in master.

@SongGuyang SongGuyang added the tests-ok The tagger certifies test failures are unrelated and assumes personal liability. label Jul 25, 2022
@SongGuyang SongGuyang merged commit bf97a69 into ray-project:master Jul 25, 2022
Rohan138 pushed a commit to Rohan138/ray that referenced this pull request Jul 28, 2022
Stefan-1313 pushed a commit to Stefan-1313/ray_mod that referenced this pull request Aug 18, 2022
Co-authored-by: 多牧 <[email protected]>
Signed-off-by: Stefan van der Kleij <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests-ok The tagger certifies test failures are unrelated and assumes personal liability.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants