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
Upgrade instance and test view to MUI #474
Upgrade instance and test view to MUI #474
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.
Looks great @ImanMahmoudinasab, thanks again for your amazing work! Here are a few comments from my side:
The following suggestion are not part of this PR though. Please tell me what you think... We can tackle those issues in a follow up PR.
- Getting TS error in sidebar component. Are you seeing the same?
the test list will fill all the space available, and the test result will be fell bellow the test lis. This was due to inadequate space for showing both tests list and test result, specially when sidebar is open. When I decide to make them full-width on <= md screens I hadn't add the collapse button for tests list. What I will do is making the test list full width in smaller screen sizes. |
In run list, I think there shouldn't be any link inside the paper and clicking everywhere of the card should navigate user to the details of the run. This will make it easy to click on the card without thinking in which spot of card you shouldn't click because it would not go to run detail view. This was my reason for this design, if you still think it is better to keep them as links, I can change the behaviour.
Here I thought most of the time user would know the origin address (at least for me is like this ;) ) so instead of showing a [long] url which will get extra attention is not what is desired by users. If they need to know the origin they can , hover over it or click on it.(we can show it on the tooltip). Again I can change it if you think we should do that. |
@agoldis Comments applied. |
New features: