-
-
Notifications
You must be signed in to change notification settings - Fork 61
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
Fair models #294
Fair models #294
Conversation
Thanks for the PR : Responsiveness :
2 ) Also for the grids it doesn't get expanded or stretched down when screen size is changed , i can only see 2 column grids all the time 3 ) We have model description in the table now , I thought you recommended to add it and aggreed to add shorter version of description as well ? Do remind me that , and revisiting would be nice to have description as well 4 ) Graph is available on the API for non authenticated user as well , you can replace this
6 ) Info icon is missing for each of them , user should be able to see / understand what are those
|
|
Thanks for your review @kshitijrajsharma . TL;DR: All you have seen are implemented as designed, anything missing means it's not in the design and, therefore, will require design updates before implementation. 1 - I don't think this is a responsiveness issue. This was intentional. We decided to use a fixed height for the map and grid because we believe it would look unappealing if the map view stretched down the whole page. It's not an issue though, it's an easy fix, we can discuss this during the call. 2 - That is how it was designed. It's also an easy fix. We can discuss this in detail during the call. This is only peculiar to the Grid + Map. 3 - It was decided by @omranlm to replace it with accuracy because the description was not available earlier. And we changed the design to that. We will need to revisit the design before implementing this. 4 - Graph has been integratd already, you can reload your page to see the recent update. 5 - Oh, okay. This is not a problem, we would need to update the design for this. We can discuss this in detail during the call. 6 - No tooltip text's that's why they're missing. When the tooltip content is populated in the 7 - What do you mean by base model information? 8 - I have already taken care of this for all images. I added a loading state that would show up if an image is too slow and also a fallback image if there is an error while loading the image. If you don't see the loading skeleton, that means the image is loaded successfully but a bit large to render. If there is an error, you'll see the fallback image from @omranlm . I hope these clarifies your questions. Thanks. |
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, thanks
What does this PR do ?
This PR is a complete implementation of the fAIr Models list and details page. It includes the following features:
How to test ?
Visit: https://f-a-ir-emmanuels-projects-1886bda9.vercel.app/models