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 ADR about VEDA V2 refactor #875

Merged
merged 10 commits into from
Mar 19, 2024
Merged

Conversation

sandrahoang686
Copy link
Collaborator

@sandrahoang686 sandrahoang686 commented Mar 6, 2024

Closes: #865

Copy link

netlify bot commented Mar 6, 2024

Deploy Preview for veda-ui ready!

Name Link
🔨 Latest commit b469b21
🔍 Latest deploy log https://app.netlify.com/sites/veda-ui/deploys/65f9347b3f52c00008e2b5d5
😎 Deploy Preview https://deploy-preview-875--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.

@sandrahoang686 sandrahoang686 changed the title Adding veda v2 refactor content Add VEDA V2 Refactor ADR to docs Mar 6, 2024
@sandrahoang686 sandrahoang686 marked this pull request as ready for review March 6, 2024 19:17
The core UI library should also expose smaller reusable components like date pickers, form elements, analysis tools, etc… (“presentational/dumb” components). This way developers/stakeholders for example can compose a page view with the A&E feature from the core UI library and consume other reusable components to construct their page view. This also now allows developers to build their own custom components on the instance side and directly incorporate them.
#### 4. Create a Data layer that manages all data fetching
The proposed data layer is designed to handle all data fetching, including STAC calls, and MDX parsing. Think of it as a versatile data utilities library. Introducing this layer would enable components in the core UI library to remain data-agnostic. In the event of scaling to additional or different data sources, expansion can be easily accomplished by integrating them into this centralized layer. Smart components (larger and stateful) would then efficiently consume this data layer.
> **Integrating with [stac-react](https://github.com/developmentseed/stac-react/tree/main)**: Because these React hooks manage data fetching to the STAC API using the Context API provider pattern, this could just be used directly as it is already its own data layer. We would have to decide where we wrap the Provider though. Ideally, at the instance level, we would wrap the provider at the highest level in the tree either around the router or specific view containers and then consume these hooks down the hierarchy on the instance side. The components in the core ui library would be prop driven so this way they can be truly data agnostic and accept whatever data passed in.
Copy link
Collaborator Author

@sandrahoang686 sandrahoang686 Mar 6, 2024

Choose a reason for hiding this comment

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

@oliverroick @sharkinsspatial in this section, I speak in a little more detail about having a data layer and prop driven data agnostic core ui library which seems to align with what was spoken about in the STAC UI Planning mtg hopefully helpful

docs/adr/veda-v2-refactor.md Outdated Show resolved Hide resolved

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
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.

I am not sure the distinction between refactor and rewrite actually makes sense, @sandrahoang686. Even if we refactor - i.e. turn individual features into (compositions of) components - the instance apps will probably be built pretty much from scratch, right?

So, should we remove the fourth option?

@sandrahoang686
Copy link
Collaborator Author

sandrahoang686 commented Mar 12, 2024

I am not sure the distinction between refactor and rewrite actually makes sense, @sandrahoang686. Even if we refactor - i.e. turn individual features into (compositions of) components - the instance apps will probably be built pretty much from scratch, right?

So, should we remove the fourth option?

@j08lue Yeah, I guess there are two parts here. We will be refactoring the core ui logic library / component library (breaking out into core feature components, modularizing for smaller re-useable components, abstracting out data logic inside the current code repo) and then for our instances, we would be rewriting the codebase because of the architecture changes.

Option 4 was about rewriting the entire core ui logic library from scratch as well which was considered but quickly scrapped.. so I can remove it!

It might make more sense for Option 3 to say [3] Refactor component library + replace design system + rewrite instances?

@j08lue
Copy link
Contributor

j08lue commented Mar 12, 2024

Option 4 was about rewriting the entire core ui logic library from scratch as well which was considered but quickly scrapped.. so I can remove it!

I see. Let us keep the option then, to show that we considered it (and why we rejected it).

It might make more sense for Option 3 to say [3] Refactor component library + replace design system + rewrite instances?

Yes, let us add that.

@sandrahoang686 sandrahoang686 force-pushed the docs/add-veda2-refactor-adr branch from 140dd9a to 971cd3a Compare March 13, 2024 15:46
Copy link
Collaborator

@hanbyul-here hanbyul-here left a comment

Choose a reason for hiding this comment

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

Thanks for working on this ADR 🙇 I left some comments!

#### 3. Modularize and create smaller reusable components
The core UI library should also expose smaller reusable components like date pickers, form elements, analysis tools, etc… (“presentational/dumb” components). This way developers/stakeholders for example can compose a page view with the A&E feature from the core UI library and consume other reusable components to construct their page view. This also now allows developers to build their own custom components on the instance side and directly incorporate them.

#### 4. Create a Data layer that manages all data fetching
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd like to clarify my understanding to ensure I've grasped this correctly.

This data utility will be consumed at the instance level and will pass the 'reconciled' data to the component (in JSON format, I think). Additionally, the utility will be optional, allowing those with a CMS system different from MDX the flexibility to format their data independently and then pass the finalized JSON to the components. In other words, we will only offer data utility for MDX files with specific frontmatter.

If my understanding aligns with the intended implementation, could we possibly integrate this explanation into the content?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think I understand what you are saying here so hopefully this explanation helps to clarify. When it comes to MDX files and parsing, the data layer will offer utilities that will help parse these and this data will be passed in as props. The component library wouldn't care where or how this data was parsed but it should just expect a typed interface. All data utilities will be optional, the library just offers the methods and the user will get to put it together. Our template instance though should model putting much of these pieces together though so there isn't so much ambiguity. Is this similar to your thoughts?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Yes it is. I mainly wanted to make it clear that we are not going to offer various types of contents parser, other than MDX 👍

j08lue added 2 commits March 14, 2024 21:17

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
@j08lue j08lue changed the title Add VEDA V2 Refactor ADR to docs Add ADR about VEDA V2 refactor Mar 15, 2024
Copy link
Collaborator

@hanbyul-here hanbyul-here left a comment

Choose a reason for hiding this comment

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

Thanks for working on it 🙇

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
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.

Thanks for the thorough work on this. I think this is a good record for our decision now. With approval from all tagged approvers, this is good to merge, @sandrahoang686. 🙏

@sandrahoang686 sandrahoang686 merged commit 8804ee3 into main Mar 19, 2024
8 checks passed
@sandrahoang686 sandrahoang686 deleted the docs/add-veda2-refactor-adr branch March 19, 2024 16:08
hanbyul-here added a commit that referenced this pull request Mar 25, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
## 🎉 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
hanbyul-here added a commit that referenced this pull request May 1, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
## 🎉 Features
- Zoom in AOI, TOI when analysis is run in
#906
- Add custom javascript injection
#846
- ADR for V2 Refactor: #875
- Open all external links in a new tab in
#870
- Include dataset id to filter layers in
#910
- Some datasets can only be analyzed with layers from the same source in
#913
- Create minimal partial data layer scaffold starting off with Data
Catalog for VEDA2 Refactor in
#893
- Add analysis preset in #921

## 🚀 Improvements
- Chart style improvement in
#903
- Data Catalog enhancement with floating filter sidebar in
#918
- Sum as statistics option in
#925
-
## 🐛 Fixes
- Sort featured stories based on publication date in descending order in
#907
- Replace latency with temporalResolution in layer info in
#898
- Add a workaround for Safari scroll problem in
#909
- Handle empty result in #922
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.

[VEDA2 Refactor] Create ADR Document for decisions around Refactor
4 participants