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

Chore/tableOfContents-safeguard #1565

Merged
merged 5 commits into from May 29, 2023

Conversation

ryceg
Copy link
Contributor

@ryceg ryceg commented May 25, 2023

Before submitting the PR:

  • Does your PR reference an issue? If not, please chat to the team on Discord or GitHub before submission.
  • Did you update and run tests before submission using npm run test?
  • Does your branch follow our naming convention? If not, please amend the branch name using branch -m new-branch-name
  • Did you update documentation related to your new feature or changes?

What does your PR address?

  • Adds a test for truthiness to prevent the elemTarget from being scrollIntoView'd if it is somehow not present
  • Changes the allowedHeadingsList to be a NodeListOf<HTMLElement> | undefined which is more accurate to its typing- it was being initialised as an empty array, and while NodeLists are able to iterate with forEach(), they are not technically arrays (src: https://developer.mozilla.org/en-US/docs/Web/API/NodeList)

@vercel
Copy link

vercel bot commented May 25, 2023

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

Name Status Preview Comments Updated (UTC)
skeleton-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 29, 2023 7:16am

@changeset-bot
Copy link

changeset-bot bot commented May 26, 2023

🦋 Changeset detected

Latest commit: 0c18af1

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@skeletonlabs/skeleton Patch

Not sure what this means? Click here to learn what changesets are.

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

@endigo9740
Copy link
Contributor

@ryceg I'm good to merge this, but we'll need a changeset submitted and the linting is failing per the CI process. You may need top pull in the latest changes per Adrians's suggestion on Discord, then:

  • run pnpm changeset to create a changeset
  • run pnpm format to run Prettier's auto-formatter for linting

@endigo9740
Copy link
Contributor

@ryceg per your pending PRs, I noted linting is failing for several of them.

However, check this out locally it appears to be the issue you're working to solve in each individual PR

/Users/chris/Development/skeleton/skeleton/sites/skeleton.dev/src/lib/components/DocsIcon/icons.ts
  7:21  warning  Unexpected any. Specify a different type  @typescript-eslint/no-explicit-any

Given this, I'm going to go ahead and merge your PRs that now contain changesets. Then I'll aim to test again once all are in place. I'd welcome your help in resolving any remaining after this.

@endigo9740 endigo9740 merged commit 6a10b5a into skeletonlabs:dev May 29, 2023
3 of 4 checks passed
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.

None yet

2 participants