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

Update icon page link to ensure proper navigation #423

Closed
wants to merge 4 commits into from

Conversation

rtibbles
Copy link
Member

Description

Currently clicking on the icons page link in the KIcon page takes you to a 404 - this does not happen when running locally though.

Updates the href to be an absolute path to ensure it navigates to the correct place.

@@ -2,7 +2,7 @@

<DocsPageTemplate apiDocs>
<DocsPageSection title="Overview" anchor="#overview">
See <DocsInternalLink href="icons" text="the page on icons" /> for design guidance and a list of available icons. If an invalid icon is used, in development Vue.js validation will warn about the error. The icon will display as a broken image <KIcon icon="brokenImage" /> icon.
See <DocsInternalLink href="/icons" text="the page on icons" /> for design guidance and a list of available icons. If an invalid icon is used, in development Vue.js validation will warn about the error. The icon will display as a broken image <KIcon icon="brokenImage" /> icon.
Copy link
Contributor

@indirectlylit indirectlylit Feb 15, 2023

Choose a reason for hiding this comment

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

There are (at least) a couple other places where this issue seems to exist:

For design guidance, see the page on <DocsInternalLink href="textfields" text="text fields" />.

For design guidance, see the page on <DocsInternalLink href="modals" text="modals" />.

Could possibly add some validation and/or fallback behavior to the DocsInternalLink component? Looks like most href attributes include a preceding / and this is the technically correct syntax.

Copy link
Contributor

Choose a reason for hiding this comment

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

This change was lost. Added a PR here: #462

@MisRob MisRob changed the base branch from release-v1.4.x to develop September 28, 2023 14:53
@MisRob
Copy link
Member

MisRob commented Sep 28, 2023

Will need to be rebased on top of the latest develop to get the new changelog. Also, please update it (the check doesn't work here yet, but perhaps it will start after the rebase)

@rtibbles
Copy link
Member Author

Is there a reason this is targeted to develop rather than release? Bad handling of missing icons is one of the most common errors we see emanating from KDS on Sentry.

@rtibbles
Copy link
Member Author

Sorry, scratch that - the extra commits here are the result of unmerged work from 1.4.x, attempting to resolve here: #456

@MisRob
Copy link
Member

MisRob commented Sep 28, 2023

Is there a reason this is targeted to develop rather than release?

No, it was just uninformed best guess based on the age of this PR.

the extra commits here are the result of unmerged work from 1.4.x, attempting to resolve here

Ah, thank you. Should we close this one then?

@MisRob
Copy link
Member

MisRob commented Sep 28, 2023

Merged #456

@MisRob MisRob closed this Sep 28, 2023
@indirectlylit indirectlylit mentioned this pull request Oct 4, 2023
4 tasks
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.

4 participants