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: Transcript Page feedback #70

Open
wants to merge 1 commit into
base: staging
Choose a base branch
from

Conversation

0tuedon
Copy link
Collaborator

@0tuedon 0tuedon commented Dec 12, 2024

To Test,
Link

This PR also closes #71

@0tuedon 0tuedon marked this pull request as draft December 12, 2024 20:40
Copy link

vercel bot commented Dec 12, 2024

@0tuedon is attempting to deploy a commit to the btc-knowledge-base Team on Vercel.

A member of the Team first needs to authorize it.

@0tuedon 0tuedon force-pushed the fixes-individual-transcript branch from 266ae54 to 71fbae7 Compare December 12, 2024 22:20
@0tuedon
Copy link
Collaborator Author

0tuedon commented Dec 16, 2024

@Emmanuel-Develops

@Emmanuel-Develops Emmanuel-Develops marked this pull request as ready for review December 16, 2024 09:38
Copy link

vercel bot commented Dec 16, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
registry ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 17, 2024 3:20pm

@0tuedon 0tuedon changed the title fix: issues with nested list showing dark gray fix: Transcript Page feedback Dec 17, 2024
@0tuedon 0tuedon force-pushed the fixes-individual-transcript branch from 71fbae7 to 2e2d329 Compare December 17, 2024 15:09
Comment on lines +42 to +46
const topicsWithTitles = topics.map(topic => {
const currentTopic = allTopics.find( topicData => topicData.slug === topic );
return currentTopic;
} )

Copy link
Member

Choose a reason for hiding this comment

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

Can we handle this at the Contentlayer level? Specifically, replacing topic slugs with their full names during the Contentlayer processing step. This would eliminate the need to perform the replacement every time we display topic tags, such as on the source page, where this replacement is currently not accounted for:
image

Copy link
Collaborator

Choose a reason for hiding this comment

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

@0tuedon any update on this?
is the suggestion clear?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Can we handle this at the Contentlayer level? Specifically, replacing topic slugs with their full names during the Contentlayer processing step. This would eliminate the need to perform the replacement every time we display topic tags, such as on the source page, where this replacement is currently not accounted for

But if we replace the topic slugs with their names then we need to to createSlug everytime we want a link?

Copy link
Collaborator

@Emmanuel-Develops Emmanuel-Develops Dec 18, 2024

Choose a reason for hiding this comment

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

mmm, true.

For clarity, to get the topic name do we unsluggify or we have a lookup with the actual name?
cc: @kouloumos @0tuedon

what of using an object in contentLayer for tags, something like {name: "P2P", slug: "p2p"}
or as a new computedField

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Currently what I did, which @kouloumos pointed out is to find the topics by looping through allTopics and get the name for them. This only fixes it one place at a time.

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