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

Parse cookie consent form as html #1163

Merged
merged 1 commit into from
Sep 20, 2024

Conversation

hanbyul-here
Copy link
Collaborator

@hanbyul-here hanbyul-here commented Sep 20, 2024

Related Ticket: {link related ticket here}

Description of Changes

We already have a mechanism to parse markdown to html bts. It is highly likely that users will try to use different markdown syntax if they see the link syntax. So here, I made parcel resolver to compile the cookieconsent form to html beforehand. I only did it for copy, but some questions are

  • does title also accept markdown?
  • should we have some kinds of prefixes to note that this is a markdown? (we use ::markdown prefix for mdx files, but this is configuration so it can be a bit different story.

Copy link

netlify bot commented Sep 20, 2024

Deploy Preview for veda-ui ready!

Name Link
🔨 Latest commit 84d3d62
🔍 Latest deploy log https://app.netlify.com/sites/veda-ui/deploys/66edd6050243e10008bbeda9
😎 Deploy Preview https://deploy-preview-1163--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.

@snmln
Copy link
Contributor

snmln commented Sep 20, 2024

Thank you Hanbyul! This is far cleaner than what I previously had. To answer your questions:

No I do not think we need to enable markdown in the title. To my knowledge there is no situation that we have discussed where we would need to accommodate that need. Additionally from a UX best practices, linking the title in this type of component would be a highly abnormal interaction pattern. I would recommend against it.

Adding the prefix is a good idea. Since that is an input type that the user base is familiar with we should denote that in some fashion to avoid any confusion.

@hanbyul-here
Copy link
Collaborator Author

I will merge this change now since some of the UI changes would need to be made because of this change. let's keep the conversation about the prefix in the og pr.

@hanbyul-here hanbyul-here marked this pull request as ready for review September 20, 2024 20:56
@snmln
Copy link
Contributor

snmln commented Sep 20, 2024

I will merge this change now since some of the UI changes would need to be made because of this change. let's keep the conversation about the prefix in the og pr.

Sounds good!

@hanbyul-here hanbyul-here merged commit 51d6270 into Cookie-consent-form-POC Sep 20, 2024
12 checks passed
@hanbyul-here hanbyul-here deleted the parse-cookie-consent-mdx branch September 20, 2024 21:01
@hanbyul-here hanbyul-here mentioned this pull request Oct 29, 2024
snmln added a commit that referenced this pull request Oct 30, 2024
You can find VEDA Dashboard preview link to test these changes:
NASA-IMPACT/veda-config#470

## 🎉 Features
- Cookie consent component:
#1163,
#1165,
#1127,
#1177
- Stories Hub export in #1162
- Expose As Is - PageHeader, LinkElement Prop & Logo in
#1184
## 🚀 Improvements

- Add docs for registry dev in
#1169
- Timeline date selection centering functionality in
#1181
- Deprecate old explore page, Mapbox component
#1176
- Update doc with new diagrams about general architecture in
#1195
-  Add playwright in #1139
- Pre-select AOIs that have the "selected" prop set to true in
#1215
- 
## 🐛 Fixes
- Check dastaset analysis exclude attribute before running analysis in
#1175
- Restrict dynamic colormaps to applicable layer types in
#1183
- Fix external link badge display for tools hosted under the same domain
in #1192
- Escape special characters in regex expression when searching datasets
in #1204


### New Contributors
* @snmln made their first contribution in
#1127
* @AliceR made their first contribution in
#1204
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