[2.x] Inertia.js: Add support for all Jetstream-provided views. #298

claudiodekker · 2020-10-02T20:14:58Z

This PR adds Inertia variants for all Jetstream included views. This allows a more native Inertia experience for users of this stack, and solves several issues and difficulties that makes for an all-in-all smoother experience.

For example, some Jetstream users have been asking questions on our Discord regarding difficulties they're experiencing with getting Jetstream to play well with Inertia. Specifically, the most common issue is when a session expires, as the user then gets redirected to the login page, and because that login page is a Blade view, Inertia renders it in a so-called 'error modal'.

To solve or prevent this, users should instead use Inertia for their login pages as well, and while some have gotten to the point where they're attempting to (re)configure Fortify to use Inertia views instead, this too isn't the most straight-forward thing to do.

reinink · 2020-10-02T20:42:19Z

@taylorotwell For what it's worth, I definitely think this is the right approach to go. The best Inertia.js experience is to do your whole app with Vue based page components. Session expiration "just works" this way, since users are automatically redirected back to the login page, which itself is an Inertia endpoint. I don't think I'd ever, personally, build an app that had a mix of Blade views and Vue page components. That said, this does create an extra maintenance burden on this project, since these auth views can no longer be shared between the two stacks. I feel like that's a worthwhile tradeoff though. 🤷‍♂️

That all said, the session expiration issue could be solved using an Inertia::location($url) (409) response that's called in the authentication middleware. This would force a "full page redirect" to the login page. To me this is more of a band-aid, and I'd again prefer the solution in this PR. 👍

@claudiodekker Excellent work brother, this looks great. 🙌

claudiodekker · 2020-10-02T21:10:04Z

Thanks! 👍

On a side-note (just so that I don't forget), Once laravel/fortify#107 gets tagged, I can remove the ->toResponse($request) calls from this PR as well. Since it hasn't yet, I've decided to leave them in for now.

reinink · 2020-10-02T21:17:28Z

Yup, and while we're creating todo lists, it would be great to get @inertiajs/inertia on v0.3.0, and then update all the Jetstream code to use the new event callbacks. When we make that change, we should add @inertiajs/progress to the project, since right now there isn't any loading indicators, since the NProgress CSS isn't included.

taylorotwell · 2020-10-05T19:12:46Z

@claudiodekker I've just tagged a new laravel/fortify release.

This is no longer necessary as of Fortify 1.6.1

claudiodekker · 2020-10-05T20:16:38Z

@taylorotwell Thanks! I've updated the PR. I also updated the composer.json to reflect the minimum Fortify version to be ^1.6.1 so that this PR is guaranteed to work, but let me know if you'd rather leave that unchanged.

Since 'filled' is used in the back-end, using remember: false will cause remember to pass.

m1guelpf · 2020-10-06T14:53:05Z

stubs/inertia/resources/js/Jetstream/AuthenticationCardLogo.vue

@@ -0,0 +1,8 @@
+<template>
+ <a href="/">
+ <svg class="w-16 h-16" viewBox="0 0 48 48" fill="none" xmlns="http:https://www.w3.org/2000/svg">


Couldn't we just reuse the ApplicationMark component here instead of duplicating?

Yeah, I looked at that, but it's different, and because I'm trying to replicate the other views to keep branding consistent I copied this one instead.

# Conflicts: # CHANGELOG.md

taylorotwell · 2020-10-06T21:15:11Z

@claudiodekker Does this need to be rebased or something? It seems to contain some random other commits?

claudiodekker · 2020-10-06T21:21:46Z

@taylorotwell Yeah, I merged 1.x into it, as that contains @m1guelpf's Ziggy PR changes.. As a result, master is behind on those changes.

taylorotwell · 2020-10-06T21:24:54Z

I've merged 1.x into master.

claudiodekker · 2020-10-06T21:27:40Z

@taylorotwell Fixed, thanks! 👍

taylorotwell · 2020-10-06T21:29:36Z

Sick. Will probably review and get this merged in tomorrow.

claudiodekker · 2020-10-06T21:31:51Z

Cool, let me know if there's anything else you need me to do 👍

taylorotwell · 2020-10-07T18:30:41Z

@claudiodekker One bug I've noticed so far... on two-factor code entry screen after login... entering the code and then hitting "enter" just toggles you to recovery code form instead of submitting auth code.

claudiodekker · 2020-10-07T18:32:15Z

Oof, yeah, that shouldn't happen.

I'll look at it in a bit, and will also double check the keyboard behavior of other forms.

taylorotwell · 2020-10-07T18:33:59Z

That was the only thing I hit so far. I think I've tested every form.

claudiodekker · 2020-10-07T18:38:48Z

Great, thanks for letting me know! 🙏

Having dinner right now, so it might take an hour or so before I have a fix. Want me to ping you when it's done?

taylorotwell · 2020-10-07T18:42:54Z

I think I'l have it fixed. Don't worry about it.

m1guelpf · 2020-10-07T18:44:03Z

stubs/inertia/resources/js/Pages/Auth/TwoFactorChallenge.vue

+ },
+ methods: {
+ toggleRecovery() {
+ this.recovery ^= true


Suggested change

this.recovery ^= true

this.recovery = ! this.recovery

They are using this to toggle between 0 and 1.

Wouldn't it be less confusing to use a boolean?

This is actually correct. It's a bitwise XOR that changes true to false, and false to true in this situation (hence toggle) 👍

That's actually a neat trick!

For sure 🙂 Also works in PHP, or any other C-based language for that matter AFAIK 👌

m1guelpf · 2020-10-07T18:45:03Z

stubs/inertia/resources/js/Pages/Auth/TwoFactorChallenge.vue

+ </div>
+
+ <div class="flex items-center justify-end mt-4">
+ <button @click.prevent="toggleRecovery" class="text-sm text-gray-600 hover:text-gray-900 underline cursor-pointer">


Suggested change

<button @click.prevent="toggleRecovery" class="text-sm text-gray-600 hover:text-gray-900 underline cursor-pointer">

<button type="button" @click.prevent="toggleRecovery" class="text-sm text-gray-600 hover:text-gray-900 underline cursor-pointer">

taylorotwell · 2020-10-07T18:45:48Z

stubs/inertia/resources/js/Pages/Auth/TwoFactorChallenge.vue

+ Use an authentication code
+ </template>
+ </button>
+ <jet-button class="ml-4" :class="{ 'opacity-25': form.processing }" :disabled="form.processing">


Already the default type for jet buttons.

claudiodekker added 7 commits October 2, 2020 19:45

Add Inertia-based Login

0dd6aba

Add Inertia-based Forgot Password

612ed68

Add Inertia-based Reset Password

5286692

Add Inertia-based Register

7f4b71d

Add Inertia-based Verify Email

3b25698

Add Inertia-based Two Factor Challenge

c2b027d

Cleanup

d104acf

driesvints changed the title ~~Inertia.js: Add support for all Jetstream-provided views.~~ [2/xInertia.js: Add support for all Jetstream-provided views. Oct 5, 2020

driesvints changed the title ~~[2/xInertia.js: Add support for all Jetstream-provided views.~~ [2.x] Inertia.js: Add support for all Jetstream-provided views. Oct 5, 2020

reinink mentioned this pull request Oct 5, 2020

Inertia - Expired session causes the website to be displayed in a modal #128

Closed

Remove toResponse chains

6a96cbf

This is no longer necessary as of Fortify 1.6.1

Login: Fix 'remember' bug

0ab2e45

Since 'filled' is used in the back-end, using remember: false will cause remember to pass.

m1guelpf reviewed Oct 6, 2020

View reviewed changes

claudiodekker added 2 commits October 6, 2020 22:42

Merge branch '1.x' into inertia-auth-support

1dd9b03

# Conflicts: # CHANGELOG.md

Use Ziggy for new Inertia Auth views

09517b8

Merge branch 'master' into inertia-auth-support

e25e36d

m1guelpf reviewed Oct 7, 2020

View reviewed changes

taylorotwell merged commit e25e36d into laravel:master Oct 7, 2020

claudiodekker deleted the inertia-auth-support branch October 7, 2020 19:11

claudiodekker mentioned this pull request Oct 8, 2020

[1.x] Added js directory path alias #328

Merged

ks217 mentioned this pull request Oct 29, 2020

Session expiry does not redirect to login #413

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[2.x] Inertia.js: Add support for all Jetstream-provided views. #298

[2.x] Inertia.js: Add support for all Jetstream-provided views. #298

claudiodekker commented Oct 2, 2020

reinink commented Oct 2, 2020

claudiodekker commented Oct 2, 2020 •

edited

Loading

reinink commented Oct 2, 2020

taylorotwell commented Oct 5, 2020

claudiodekker commented Oct 5, 2020 •

edited

Loading

m1guelpf Oct 6, 2020

claudiodekker Oct 6, 2020

taylorotwell commented Oct 6, 2020

claudiodekker commented Oct 6, 2020 •

edited

Loading

taylorotwell commented Oct 6, 2020

claudiodekker commented Oct 6, 2020

taylorotwell commented Oct 6, 2020

claudiodekker commented Oct 6, 2020 •

edited

Loading

taylorotwell commented Oct 7, 2020

claudiodekker commented Oct 7, 2020

taylorotwell commented Oct 7, 2020

claudiodekker commented Oct 7, 2020

taylorotwell commented Oct 7, 2020

m1guelpf Oct 7, 2020 •

edited

Loading

taylorotwell Oct 7, 2020

m1guelpf Oct 7, 2020

claudiodekker Oct 7, 2020

m1guelpf Oct 7, 2020

claudiodekker Oct 7, 2020 •

edited

Loading

m1guelpf Oct 7, 2020

This comment was marked as resolved.

taylorotwell Oct 7, 2020

	<button @click.prevent="toggleRecovery" class="text-sm text-gray-600 hover:text-gray-900 underline cursor-pointer">
	<button type="button" @click.prevent="toggleRecovery" class="text-sm text-gray-600 hover:text-gray-900 underline cursor-pointer">

[2.x] Inertia.js: Add support for all Jetstream-provided views. #298

[2.x] Inertia.js: Add support for all Jetstream-provided views. #298

Conversation

claudiodekker commented Oct 2, 2020

reinink commented Oct 2, 2020

claudiodekker commented Oct 2, 2020 • edited Loading

reinink commented Oct 2, 2020

taylorotwell commented Oct 5, 2020

claudiodekker commented Oct 5, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

taylorotwell commented Oct 6, 2020

claudiodekker commented Oct 6, 2020 • edited Loading

taylorotwell commented Oct 6, 2020

claudiodekker commented Oct 6, 2020

taylorotwell commented Oct 6, 2020

claudiodekker commented Oct 6, 2020 • edited Loading

taylorotwell commented Oct 7, 2020

claudiodekker commented Oct 7, 2020

taylorotwell commented Oct 7, 2020

claudiodekker commented Oct 7, 2020

taylorotwell commented Oct 7, 2020

m1guelpf Oct 7, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

claudiodekker Oct 7, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as resolved.

Choose a reason for hiding this comment

claudiodekker commented Oct 2, 2020 •

edited

Loading

claudiodekker commented Oct 5, 2020 •

edited

Loading

claudiodekker commented Oct 6, 2020 •

edited

Loading

claudiodekker commented Oct 6, 2020 •

edited

Loading

m1guelpf Oct 7, 2020 •

edited

Loading

claudiodekker Oct 7, 2020 •

edited

Loading