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

Add Layer info data as a layer attribute #843

Merged
merged 10 commits into from
Feb 15, 2024
Merged

Conversation

hanbyul-here
Copy link
Collaborator

@hanbyul-here hanbyul-here commented Feb 13, 2024

Part of #823

Heavily edited after the last time I opened this PR

With this pr, layer can include info attribute. info consists of

info:
  modal: 
    subtitle: |
      ::markdown subtitle in markdown
    contents: |
      ::markdown contents in markdown
  template:
        source: string
        spatialExtent: string
        latency: string
        unit: string

I added a sandbox page to trigger the modal. https://deploy-preview-843--veda-ui.netlify.app/sandbox/sandboxLayerInfo

@faustoperez Currently, there is no easy way to determine if the link the user uses is an external or an internal link. (The link is just part of the HTML that a user passes as a part of Layerdata.) So I think we might have to skip the external link icons. :[

I am not implementing the link on top of the header, since I can pick that component up from the other branch currently working on (#829)
Screen Shot 2024-02-14 at 10 48 14 AM

Details of technical implementation: This PR required an Implementation of processing markdown in frontmatter in stringfy-yml-func.

  • I thought about passing Markdown as it is and delegating the rendering of it to the front-end, but it will increase the load of front-end work at this point. (Integrating it with our current MDX system for rendering stories and datasets seems challenging.)
  • Initially considered using our system's markdown parsers (remark and rehype), but they are pure ES modules requiring rewiring in the pipeline, which I wanted to avoid in this PR. Therefore, I chose a different markdown parser.
  • It's important to note that info.modal.subtitle and info.modal.contents should only contain markdown, not MDX, to exclude custom components.

Copy link

netlify bot commented Feb 13, 2024

Deploy Preview for veda-ui ready!

Name Link
🔨 Latest commit 2c31ccf
🔍 Latest deploy log https://app.netlify.com/sites/veda-ui/deploys/65ce3c7ac3518a0008b694c2
😎 Deploy Preview https://deploy-preview-843--veda-ui.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

Copy link
Contributor

@j08lue j08lue left a comment

Choose a reason for hiding this comment

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

We need nice property titles, not just slug-like keys, please.

parcel-resolver-veda/index.d.ts Outdated Show resolved Hide resolved
mock/datasets/no2.data.mdx Outdated Show resolved Hide resolved
@hanbyul-here
Copy link
Collaborator Author

I think I poorly communicated what the code does in this PR. I will close this for now and open it again after making it clear.

Copy link
Contributor

@j08lue j08lue left a comment

Choose a reason for hiding this comment

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

Looks good and the choice of making it all Markdown is solid for now (see comment below).

image

Comment on lines 60 to 71
subtitle: |
::markdown
Global, monthly 1 km resolution dataset of fossil fuel carbon dioxide emissions, version 2022
contents: |
::markdown
- Temporal Extent: January 2000 - December 2021
- Temporal Resolution: Monthly
- Spatial Extent: Global
- Spatial Resolution: 1 km x 1 km
- Data Units: Tons of carbon per 1 km x 1 km cell (monthly total)
- Data Type: Research
- Data Latency: Updated annually, following the release of an updated [BP Statistical Review of World Energy report](https://www.bp.com/en/global/corporate/energy-economics.html)
Copy link
Contributor

Choose a reason for hiding this comment

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

All Markdown seems like a good solution for now.

At some point in the future, we will want this to be more structured (and probably coming from STAC), following the example of Planetary Computer. But free form is fine for now.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, I agree. Especially since our current application also asks for structured data (why info has a separate section for template). We would need more streamlined thoughts about how to handle this layer information before though. I can't think of a straightforward way of having both descriptive layerinfo and structured layerinfo without manual edits and duplication of the data.

</Block>
</>
) : (
<div> Cannot find layer info fron dataset no2</div>
Copy link
Collaborator

Choose a reason for hiding this comment

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

🔧 : Mispelled from and should no2 be hardcoded here?

@hanbyul-here
Copy link
Collaborator Author

I moved markdown level data to the dataset (infoDescription - please let me know if there is a better way to name these.), left info to layer level. Added these two to documentation ticket: #842

@hanbyul-here hanbyul-here merged commit 24a06b3 into main Feb 15, 2024
8 checks passed
@hanbyul-here hanbyul-here deleted the 823-layer-info-data branch February 15, 2024 16:44
hanbyul-here added a commit that referenced this pull request Mar 25, 2024
## 🎉 Features
- Optional media attributes for layers:
#843
- Add custom javascript injection
#846
- ADR for V2 Refactor: #875

## 🚀 Improvements
- E&A imporvement. Related tickets
  - Layer select modal: #845
- Connect dataset information on layer:
#821
  - Layer info modal: #849
- Update data layer card:
#851
  - Hidden layers: #867
- Fast follow-ups: #851 ,
#862,
#863,
#860
  - PR template: #880


## 🐛 Fixes
- Return datasets even when there is a dataset without summaries:
#786
- Show all the datasets on Data Catalog page:
#837
- Block Map user defined position fix:
#784
- Geocoder centering on various projecctions:
#826
- Wording, typo: #869
#854,
#874,
- Fix yaxis labeling: #883
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants