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

Add Support for Dynamic Title Updates in SEO Package for InertiaJS #89

Merged
merged 8 commits into from
Jan 20, 2025

Conversation

touqeershafi
Copy link
Contributor

Background

This pull request addresses the issue of missing dynamic title updates in the SEO package when using InertiaJS.

Changes Made:

Introduced a new configuration option for the title tag that allows for custom attributes, enabling dynamic updates during page transitions.
Updated the README.md to clarify the automatic rendering of tags and the ability to add custom attributes as needed.

Motivation:

Dynamic title updates are essential for improving user experience and SEO. This enhancement ensures that the title tag reflects the current page context without requiring manual management.

Related Issue:

InertiaJS: Title Attribute Fails to Support Dynamic Title Changes #88

This PR relates to the discussion found here: InertiaJS Issue #978.

@ralphjsmit
Copy link
Owner

Hello! Thank you for your PR, I like the idea! Just a question, would it be an idea to make this inertia attribute be added automatically to the title tag, based on some if-condition whether Inertia is currently rendering? Then people don't have to think about enabling this.

@touqeershafi
Copy link
Contributor Author

To determine if the Laravel app supports Inertia.js, the most effective method I can suggest is to check the Inertia middleware. This can be achieved by using Route::gatherRouteMiddleware(Route::current()) to retrieve the list of current route middleware and then verifying whether the Inertia.js middleware is included.

For reference, the Inertia.js middleware is detailed in the backend setup documentation: Inertia.js Server-Side Setup. However, If a different namespace is used for the middleware, this approach will not work.

If you have any other suggestions or methods, I’d be open to hearing them.

@ralphjsmit
Copy link
Owner

Good idea, I like that! Then I would suggest just iterate through the middleware, for each perform a check with the is_subclass_of($middleware, \Inertia\Middleware::class) and then it should be good for everyone I think.

@touqeershafi
Copy link
Contributor Author

@ralphjsmit I successfully implemented what you suggested. However, I believe it's not the most efficient solution, as it requires iterating over all the middlewares for the route. If you're comfortable with this approach, I can go ahead and push the changes. Still, I think using the configuration method would be a better approach. Ultimately, it depends on your preference.

$currentMiddleware = collect(Route::gatherRouteMiddleware(Route::current()));
$hasInertiaMiddleware = $currentMiddleware->contains(function ($middleware) {
    return is_subclass_of($middleware, "Inertia\Middleware");
});

if($hasInertiaMiddleware) {
    $this->attributes = ['inertia' => ""];
}

@ralphjsmit
Copy link
Owner

Hey @touqeershafi, thanks for testing, looks good! In terms of performance, I just ran a benchmark and it took me just about 0.021ms running locally, so I think that's quite okay.

@ralphjsmit ralphjsmit merged commit 3dfffdf into ralphjsmit:main Jan 20, 2025
17 of 18 checks passed
@ralphjsmit
Copy link
Owner

I added some tests and just released in 1.6.5. Thanks!

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.

2 participants