-
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
base: main
Are you sure you want to change the base?
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
src/App.tsx
Outdated
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.
You should look at how the main branch deals with utilizing main.tsx! We decided to add in new pages for changelog and documentation so this will be helpful in understanding the frontend structure :)
(so basically we are not using App.tsx anymore and it should be deleted!)
src/components/DataTable.tsx
Outdated
<Input.Wrapper> | ||
<Textarea | ||
variant="unstyled" | ||
value=" |
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 this back to file.note
src/components/DataTable.tsx
Outdated
style={{ whiteSpace: "normal", wordWrap: "break-word" }} | ||
readOnly | ||
placeholder="Note" | ||
rows={4} |
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 you avoid hard coding the amount of rows? Maybe make it dynamic?
src/components/DataTable.tsx
Outdated
// now with four rows rows rows rows rows rows rows rows rows rows rows rows rows rows rows rows rows rows rows rows" | ||
style={{ whiteSpace: "normal", wordWrap: "break-word" }} | ||
readOnly | ||
placeholder="Note" |
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.
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!
<Table.Td style={{ paddingRight: "25px" }}> | ||
<Input.Wrapper> | ||
<Textarea | ||
variant="unstyled" |
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 you add size="xs" to this as well as the location?
<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?
} | ||
|
||
.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)
@@ -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!
@@ -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
No description provided.