Data Catalog filters/search

[danielfdsilva]

Implement Data Catalog page according with the mockups (#484)

• Date indication on the card and date sorting options were not implemented (See Modified date on data catalog datasets #514 for context)
• Source is currently missing, therefore all cards day By SOURCE and the source filter only has the All options (More info on Implement data catalog new page (filtering / search) #511 )

[netlify]

✅ Deploy Preview for veda-ui ready!

Name	Link
🔨 Latest commit	12f3184
🔍 Latest deploy log	https://app.netlify.com/sites/veda-ui/deploys/644a970158cde50008a827cc
😎 Deploy Preview	https://deploy-preview-508--veda-ui.netlify.app/
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[nerik]

Looking great! Love the attention to details like the collapsible search field on mobile 🥳

A bunch of things:

• I had to uncomment the thematics field in the urban heating dataset for this to run.
• (nit) cursor on the featured datasets card's thematic pill is reverting to the 'move' cursor on hover, and it has a hover style (while not clickable) which is a bit confusing. Also because it's not clickable, I'd give it a disabled style especially since the pills on the non featured cards are clickable.
• Clicking on the card's thematic pills should scroll up to the filters bar IMHO.
• Make search case-insensitive ('Car' yields results, 'car' does not)
• We probably don't need the card's top right 'DATASET' pill
• (nice-to-have) Also perform a search on thematic areas ('eis' in the search box should filter datasets with eis thematic)
• (nice-to-have) On the dataset count block:
  'Showing 2 datasets out of 45 clear filters' or 'Showing all 45 datasets'

[nerik]

Just out of curiosity: do we have a set practice on using React types directly, rather than importing them (import React, { ReactNode } from 'react';)

[danielfdsilva]

I don't think we do. Do you favor one?

[nerik]

We usually import everything at the top of a file, so for consistency's sake, I don't see why this should be different for React types (especially in this file where we have 4 occurrences). Very much nitpicking though.

[danielfdsilva]

@nerik Did this in a separate PR because there were a lot of changes. #517

[nerik]

Might wanna memoize that?

[danielfdsilva]

👍

[danielfdsilva]

Thank you for the very thorough review @nerik! 🙏

Some notes on your points:

I had to uncomment the thematics field (...)

I made the thematics optional for now, but should they be required on every dataset?
Currently the Urban heating doesn't have them because it was a way to hide it (Can't find where this convo happened). They didn't want to create a dataset page but wanted to use it in a discovery. Should this be a thing? I'd vote against.

cursor on the featured datasets (...)

Pill now behaves like any other text. I decided not to disabled it running the risk of making it too invisible. @ricardoduplos, any concerns on this point?

Clicking on the card's thematic pills should scroll up to the filters bar IMHO.

Great idea! Done.

Make search case-insensitive ('Car' yields results, 'car' does not)

Done

We probably don't need the card's top right 'DATASET' pill

There are places where there is related data (like discoveries) and the cards show up. That pill is the way to tell them apart. To be consistent it is also shown here. I think @ricardoduplos was rethinking that, but maybe for later.

Also perform a search on thematic areas (...)

Wouldn't this make the filter redundant? Or is this good redundancy? @ricardoduplos thoughts?

'Showing 2 datasets out of 45 clear filters' (...)

Implemented. @ricardoduplos to comment on design.

Add hint to indicate needing 3 chars on txt search

@ricardoduplos Would the UI library form field hint work here?

[nerik]

Thanks for addressing the feedback, and sorry for not commenting in-context, this makes things messy.

I made the thematics optional for now, but should they be required on every dataset?
Currently the Urban heating doesn't have them because it was a way to hide it (Can't find where this convo happened). They didn't want to create a dataset page but wanted to use it in a discovery. Should this be a thing? I'd vote against.

I'm not sure but ideally, we'd have an explicit hideInCatalog or similar for something like that.

We probably don't need the card's top right 'DATASET' pill
There are places where there is related data (like discoveries) and the cards show up. That pill is the way to tell them apart. To be consistent it is also shown here. I think @ricardoduplos was rethinking that, but maybe for later.

Also deferring to @ricardoduplos - if we leave it I'd recommend disabling the hover styles similar to what you did for the thematic pills.

[danielfdsilva]

@nerik @ricardoduplos Addressed all the items we discussed. :) See commits above

[danielfdsilva]

@karitotp This is ready to undergo some QA testing.
If you want to use production data you'll need to checkout the feature/remove-thematic-areas branch from veda-config which includes the needed changes in data.

⚠️ ⚠️ We're still waiting for the dataset source information so that part is not expected to work.