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

ui: indicate direction on node latency table #33299

Closed
wants to merge 1 commit into from

Conversation

tbg
Copy link
Member

@tbg tbg commented Dec 20, 2018

Release note: None

@tbg tbg requested a review from a team December 20, 2018 13:29
@cockroach-teamcity
Copy link
Member

This change is Reviewable

@tbg tbg requested a review from couchand December 21, 2018 09:17
Copy link
Contributor

@couchand couchand left a comment

Choose a reason for hiding this comment

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

Thanks for knocking this out @tbg! I've always forgotten which direction is which on that table.

cc @piyush-singh let's take this into account when we bring this into the main pages

function createHeaderCell(staleIDs: Set<number>, id: Identity, key: string) {
const node = `n${id.nodeID.toString()}`;
function createHeaderCell(staleIDs: Set<number>, id: Identity, key: string, titlePrefix: string) {
const node = titlePrefix + `n${id.nodeID.toString()}`;
Copy link
Contributor

Choose a reason for hiding this comment

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

style nit: add to the interpolated string:

const node = `${titlePrefix}n${id.nodeID.toString()}`

@@ -285,14 +285,15 @@ class Network extends React.Component<NetworkProps, {}> {
staleIDs,
identity,
`0-${identity.nodeID}`,
`to `,
Copy link
Contributor

Choose a reason for hiding this comment

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

style nit: just use a plain string: 'to '

))
}
</tr>
{
_.map(displayIdentities, (identityA) => (
<tr key={identityA.nodeID} className="network-table__row">
{
createHeaderCell(staleIDs, identityA, `${identityA.nodeID}-0`)
createHeaderCell(staleIDs, identityA, `${identityA.nodeID}-0`, ``)
Copy link
Contributor

Choose a reason for hiding this comment

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

style nit: maybe a default parameter, or at least use a plain string: ''?

@tbg tbg added the X-noremind Bots won't notify about PRs with X-noremind label Jun 19, 2019
@tbg tbg closed this Oct 1, 2020
@tbg tbg deleted the ui/latency-direction branch October 1, 2020 12:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
X-noremind Bots won't notify about PRs with X-noremind
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants