-
Notifications
You must be signed in to change notification settings - Fork 1
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
fixed table sizing with length of notes #68
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you resolve merge conflicts? and the branch not being deployed is a bit weird, you might want to look at it!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall, Very Good! I'm so proud of you :))
Just tiny changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I reviewed the page with the leads, and they made some suggestions for us! I made some comments that go over what was discussed! You're doing great, they love how it looks xD!
src/components/DataTable.tsx
Outdated
<Table.Td>{getFileNameWithoutExtension(file.mcap_files[0].file_name)}</Table.Td> | ||
<Table.Td style={{ paddingLeft: "25px" }}> | ||
{getFileNameWithoutExtension(file.mcap_files[0].file_name)} | ||
</Table.Td> | ||
<Table.Td>{file.date}</Table.Td> | ||
<Table.Td>{file.location}</Table.Td> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can u do the same for location but with maxRows of 1?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think server said it was down ? but even though my channels are activated, no data points are loading so im not sure what this looks like. i used the same style for file.notes, but im not sure what it looks liek on the frontend since the data hasnt been loading since yesterday
src/css/Root.css
Outdated
} | ||
|
||
.table-contain-result { | ||
flex-grow: 1; | ||
overflow-y: auto; | ||
padding-bottom: 10px; | ||
} | ||
width: 80%; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can u make this 60% (also min-width)
src/css/SearchBar.css
Outdated
@@ -4,7 +4,7 @@ | |||
font-size: 15px; | |||
overflow: auto; | |||
padding-bottom: 10px; | |||
border-left: solid #D1BF80 2px; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change back!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I thought SearchBar.css has stayed the same for a while? What should this be changed back to?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh nvm i found it
package.json
Outdated
@@ -21,7 +21,7 @@ | |||
"@mantine/hooks": "^7.13.1", | |||
"@tabler/icons-react": "3.17.0", | |||
"dayjs": "^1.11.13", | |||
"nuqs": "^2.2.1", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might want to change this back. Im not sure if this was the exact issue but application might be loading slower because of this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
No description provided.