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

Fix prop type for Map and Scrollytelling #1361

Merged
merged 2 commits into from
Jan 8, 2025

Conversation

hanbyul-here
Copy link
Collaborator

@hanbyul-here hanbyul-here commented Jan 7, 2025

Related Ticket: #1324

Description of Changes

While contemplating on what @sandrahoang686 mentioned about the data proptype, I realized that our types are wrong for Map and Scrollytelling (And We have different types, but named same of DatasetData which we should address soon.)

Notes & Questions About Changes

Map and Scrollytellinglblock seems to expect the same shape of the dataset which is an object (VedaData<DatasetData> - am I missing anything?

Copy link

netlify bot commented Jan 7, 2025

Deploy Preview for veda-ui ready!

Name Link
🔨 Latest commit db6eff4
🔍 Latest deploy log https://app.netlify.com/sites/veda-ui/deploys/677eec2d864eef0008f37135
😎 Deploy Preview https://deploy-preview-1361--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
Copy link
Collaborator

Thanks @hanbyul-here for catching this 🙇🏼‍♀️. I pushed up a change though to just rename the type which makes more sense to do. So the type could be more informative instead of just giving it a new type alias in different files. I'm going to approve and merge in.

Also, We are going to need to do a general cleanup by the way between these two files because the /exploration/types.d.ts.ts references some types in $types/veda.ts. For example EnhancedDatasetLayer is duplicated between exploration types here and veda types here. Smalls things like ParentDatset is mispelled so we should probably just take a good sweeping look between the two.

We also should not be casting here. Casting tends to mask a deeper issue with our types and having bad types or mis-transformations which means we should revisit our types here again.

Created ticket for the tech-debt here => #1363

@sandrahoang686 sandrahoang686 merged commit 93ae948 into 1324-expose-mdx-components-1 Jan 8, 2025
9 checks passed
@sandrahoang686 sandrahoang686 deleted the fix-vedadata-type branch January 8, 2025 21:34
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.

2 participants