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

[feature request] Allow custom IDs #339

Closed
GregBrimble opened this issue Sep 19, 2024 · 2 comments · Fixed by #340
Closed

[feature request] Allow custom IDs #339

GregBrimble opened this issue Sep 19, 2024 · 2 comments · Fixed by #340
Assignees
Labels
enhancement New feature or request released

Comments

@GregBrimble
Copy link
Contributor

GregBrimble commented Sep 19, 2024

Hi! 👋

Firstly, thanks for your work on this project! 🙂

Today I used patch-package to patch [email protected] for the project I'm working on.

We use Astro Breadcrumbs in the root layout of our pages, but only in that one place (we have a lot of control over it). Because Astro Breadcrumbs uses a UUID for the ID, this means that every single page changes on every new build. Some platforms, like Cloudflare Pages, optimize their deployment process by only uploading changed files, and so, by using Astro Breadcrumbs, we're in a place where every single file is changing and so the entire website needs to be uploaded fresh, every time. Allowing us to customize the ID of Astro Breadcrumbs component would allow us to create deterministic pages and would speed up our deployment process massively!

Here is the diff that solved my problem:

diff --git a/node_modules/astro-breadcrumbs/src/Breadcrumbs.astro b/node_modules/astro-breadcrumbs/src/Breadcrumbs.astro
index b4a60ee..25fc657 100644
--- a/node_modules/astro-breadcrumbs/src/Breadcrumbs.astro
+++ b/node_modules/astro-breadcrumbs/src/Breadcrumbs.astro
@@ -28,12 +28,12 @@ const {
   excludeCurrentPage = false,
   debug = false,
   separatorAriaHidden = true,
+	id = crypto.randomUUID(),
 } = Astro.props as BreadcrumbsProps;
 
 const paths = Astro.url.pathname.split("/").filter((crumb: any) => crumb);
 const hasTrailingSlash = Astro.url.pathname.endsWith("/");
 const pathLength = paths?.length;
-const UUID = crypto.randomUUID();
 const listCssClasses = [
   `${mainBemClass}__crumbs`,
   Astro.slots.has("separator") ? " has-separators" : " has-no-separators",
@@ -63,11 +63,11 @@ debugInformation(debug, parts, customizedParts);
 
 <astro-breadcrumbs
   data-main-bem-class={mainBemClass}
-  data-id={UUID}
+  data-id={id}
   data-path-length={pathLength}
   data-truncated={truncated}
 >
-  <nav aria-label={ariaLabel} class={mainBemClass} id={UUID} {...customizeNav}>
+  <nav aria-label={ariaLabel} class={mainBemClass} id={id} {...customizeNav}>
     <ol class:list={listCssClasses} {...customizeList}>
       {
         processedParts.map(
diff --git a/node_modules/astro-breadcrumbs/src/breadcrumbs.types.ts b/node_modules/astro-breadcrumbs/src/breadcrumbs.types.ts
index 019011e..5459a37 100644
--- a/node_modules/astro-breadcrumbs/src/breadcrumbs.types.ts
+++ b/node_modules/astro-breadcrumbs/src/breadcrumbs.types.ts
@@ -15,6 +15,7 @@ export interface BreadcrumbsProps {
   excludeCurrentPage?: boolean;
   debug?: boolean;
   separatorAriaHidden?: boolean;
+	id?: string;
 }
 
 export interface CustomizeElement extends AddAttributes {

More than happy to put up a PR with these changes, if you'd support this change. (Edit: I just went ahead and did that in #340 given how small of a change this is.) Thanks again!

This issue body was partially generated by patch-package.

@felix-berlin
Copy link
Owner

Hi @GregBrimble ,
I'm glad that this project is helping you at Cloudflare.

While browsing through your docs I noticed at some point that you use astro-breadcrumbs. Really great!

Thanks also for the pull request, it looks good. Your change with the custum IDs can be adopted.

@felix-berlin
Copy link
Owner

🎉 This issue has been resolved in version 3.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request released
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants