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

Fair models #294

Merged
merged 28 commits into from
Oct 21, 2024
Merged

Fair models #294

merged 28 commits into from
Oct 21, 2024

Conversation

jeafreezy
Copy link
Collaborator

What does this PR do ?

This PR is a complete implementation of the fAIr Models list and details page. It includes the following features:

  • Map view.
  • List view.
  • Grid view.
  • Multi-View (Grid + Map).
  • Supports Date Range Filtering.
  • Support Sorting.
  • Supports Searching.
  • Supports Mapview Filtering using Model ID.
  • It keeps filtering states in the URL params for easy sharing.
  • Models card - (Model workspace files, training status, feedback, and logs.).
  • Pagination.
  • Mobile responsiveness.
  • and many more.

How to test ?

Visit: https://f-a-ir-emmanuels-projects-1886bda9.vercel.app/models

@kshitijrajsharma
Copy link
Member

Thanks for the PR :
Few Issues I noticed :

Responsiveness :

  • This is on map view , only grid is working
  1. Model page is not responsive for the big screens
    image
    It has this white space and always sticks to same size

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
image
Check reference here : https://tasks.hotosm.org/explore?omitMapResults=0
Similar issue here on tablet size , grids are not being stiched up properly
image

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
image

4 ) Graph is available on the API for non authenticated user as well , you can replace this
image

  1. Would be good to add copy option so that user can copy the url directly as it might not make sense to open it in browser because it would do nothing !
    image

  2. Would be nice to have different indication like in table for failed submitted and running
    image

  • we also need that hierarchy in each training ,it can be labled as training files , each checkpoints are available to download / view !
  • same goes for the training area , training area should be visible for each checkpoints too , on the same model perhaps ( this will come with the pmtiles bundle , lets keep the map for now

6 ) Info icon is missing for each of them , user should be able to see / understand what are those
image

  • we should also have info icon on history as well , user should know what history means , how to interpret the information being plotted in his/her screen

@kshitijrajsharma
Copy link
Member

  • We are missing the base model information on the models page !

@kshitijrajsharma
Copy link
Member

When I click on the different models in my map , it is taking some time to render the image which is fine may be we can add that loading thingy on the thumbnail , or whichever is best from user experience perspective !
image

@jeafreezy
Copy link
Collaborator Author

Thanks for the PR : Few Issues I noticed :

Responsiveness :

  • This is on map view , only grid is working
  1. Model page is not responsive for the big screens
    image
    It has this white space and always sticks to same size

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 image Check reference here : https://tasks.hotosm.org/explore?omitMapResults=0 Similar issue here on tablet size , grids are not being stiched up properly image

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 image

4 ) Graph is available on the API for non authenticated user as well , you can replace this image

  1. Would be good to add copy option so that user can copy the url directly as it might not make sense to open it in browser because it would do nothing !
    image
  2. Would be nice to have different indication like in table for failed submitted and running
    image
  • we also need that hierarchy in each training ,it can be labled as training files , each checkpoints are available to download / view !
  • same goes for the training area , training area should be visible for each checkpoints too , on the same model perhaps ( this will come with the pmtiles bundle , lets keep the map for now

6 ) Info icon is missing for each of them , user should be able to see / understand what are those image

  • we should also have info icon on history as well , user should know what history means , how to interpret the information being plotted in his/her screen

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 APP_CONTENT, they'll show up.

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.

Copy link
Contributor

@omranlm omranlm left a comment

Choose a reason for hiding this comment

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

Looks great, thanks

@omranlm omranlm merged commit 7fe9277 into hotosm:develop Oct 21, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants