-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Minor frontend fixes #116
Minor frontend fixes #116
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.
The changes in general are fine!
I left some comments and questions.
<Paper | ||
sx={{ | ||
p: 4, | ||
[theme.breakpoints.up("xs")]: { ml: 8 }, | ||
[theme.breakpoints.up("sm")]: { ml: 12 }, | ||
[theme.breakpoints.up("md")]: { ml: 20 }, | ||
[theme.breakpoints.up("lg")]: { width: "80vw", ml: 15 }, | ||
[theme.breakpoints.up("xl")]: { width: "80vw", ml: 10 }, | ||
}} | ||
> |
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.
Question: Is the margin left assigned from the left edge of the image? I think it would be better if it was with respect to the drawer.
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.
Yes, the margin left is assing from the edge.
I am trying to define the margin with respect to the drawer, and I think I managed to do it. But by modifying that, now i haven't been able to make the table take up the entire space.
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 that Drawer is not the best component to use in this case (it is more intended to be the drawer where the navigation menu of the page is located) but List.
This will allow you to have the objects live in the same view and therefore, the size calculations are done with respect to the list and the table.
Suggestion: You could implement a view with two Grid
, the one on the right with a small size where the List
of experiments (ListItems
) goes and the one on the left where the visualization goes.
And then make the grid sizes variable according to the size of the page using the typical sm, md, lg, etc...
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.
It looks good!
I think Container (in app.jsx
) is the component that is causing the strange behaviors regarding the view responsive sizes.
For now, it's fine, but in the future we will have to think about how to deactivate it.
A minor margin change was requested.
Question: Could you disable the black background of the experiment list?
Summary
This PR includes small changes that were pending to be implemented in the frontend.
Type of change
Changes
ResultsPage.jsx
. The drawer no longer covers the table even if the screen width is changed.RunnerDialog.jsx
). Now the user's selection is preserved (previously, all runs were reselected on any update).How to Test
DashAI/DashAI/front
and runyarn build
DashAI/
and runpython -c "import DashAI;DashAI.run()"
Screenshots
Click to toggle the screenshots
1. Responsiveness in results page.Select.webm
edit.dataset.webm