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

Implement basic reading time functionality #91

Open
wants to merge 24 commits into
base: main
Choose a base branch
from

Conversation

trueberryless
Copy link
Contributor

@trueberryless trueberryless commented Nov 10, 2024

Describe the pull request

Add reading time after discussion #74 was left open for 3 months.

How

  • new Starlight component override: PageTitle.astro

  • add property to config.ts (showReadingTime: generall set if readingTime should be calculated) and schema (readingTime: manually override calculated readingTime or show specific readingTime if showReadingTime is deactivated)

  • additional libs file (readingTime.ts)

  • change the MarkdownContent.astro and Metadata.astro files and create PageTitle.astro file

  • If readingTime is too large it shows in hours and minutes

  • write tests for libs/readingTime.ts

Screenshots

image
image

manual override:
image

Copy link

vercel bot commented Nov 10, 2024

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

Name Status Preview Comments Updated (UTC)
starlight-blog-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 23, 2024 11:12am

Copy link

vercel bot commented Nov 10, 2024

Deployment failed with the following error:

The provided GitHub repository does not contain the requested branch or commit reference. Please ensure the repository is not empty.

@trueberryless
Copy link
Contributor Author

Okay, I'm sorry for the many commits (Vercel build minutes) HiDeoo, but I think I got the most features covered from #74

@trueberryless
Copy link
Contributor Author

trueberryless commented Nov 10, 2024

Some thinks to note down before merging:

  • there is currently the inconsistency that the reading time has a space between the number and string: 2 min or 1 h but when there are multiple numbers, like here: 1h 9min I chose not to include the space because it looks weird for me, but this is something we need to discuss. I just saw that @julien-deramond just has the spaces in every case:

    image

  • there are currently NO TESTS written by me because 1. I'm not that familiar with writing test currently (Im working on it) and 2. I dont know how you, HiDeoo, want to handle the test scenarios. I looked into the test folder and wasn't sure if I should create a new file under /basics or just some random test in utils so I leave it up to you... I have written tests for the libs/readingTime.ts

  • the documentation is expandable (basics are covered but nothing too advanced)

  • when Starlight merges [WIP] Move route data to Astro.locals withastro/starlight#2390 we'll probably have to revised all overridden components and see which we can avoid to override

not finished
Copy link

changeset-bot bot commented Dec 23, 2024

⚠️ No Changeset found

Latest commit: 440d7a9

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@trueberryless
Copy link
Contributor Author

It's probably better to wait with this feature until the route data is moved to Astro.locals in Starlight. After the refactoring of the plugin, it's easier to implement this feature...

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.

1 participant