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

fixed table sizing with length of notes #68

Merged
merged 22 commits into from
Jan 29, 2025
Merged

fixed table sizing with length of notes #68

merged 22 commits into from
Jan 29, 2025

Conversation

jyuenbeep
Copy link
Contributor

No description provided.

.env Outdated Show resolved Hide resolved
Copy link
Contributor

@aesteri aesteri left a 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!

@jyuenbeep jyuenbeep requested a review from aesteri December 17, 2024 17:13
Copy link
Contributor

@aesteri aesteri left a 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 Show resolved Hide resolved
src/components/DataTable.tsx Outdated Show resolved Hide resolved
src/components/DataTable.tsx Outdated Show resolved Hide resolved
src/components/DataTable.tsx Outdated Show resolved Hide resolved
@jyuenbeep jyuenbeep requested a review from aesteri January 8, 2025 00:37
Copy link
Contributor

@aesteri aesteri left a 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 Show resolved Hide resolved
<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>
Copy link
Contributor

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?

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 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%;
Copy link
Contributor

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;
Copy link
Contributor

Choose a reason for hiding this comment

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

Change back!

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 thought SearchBar.css has stayed the same for a while? What should this be changed back to?

Copy link
Contributor Author

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",
Copy link
Contributor

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

@aesteri aesteri self-requested a review January 28, 2025 23:43
aesteri
aesteri previously approved these changes Jan 29, 2025
Copy link
Contributor

@aesteri aesteri left a comment

Choose a reason for hiding this comment

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

LGTM!

@kaisernarkotic kaisernarkotic merged commit 9e76094 into main Jan 29, 2025
1 check passed
@kaisernarkotic kaisernarkotic deleted the scroll-ui branch January 29, 2025 00:22
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.

3 participants