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 Netlify identity widget on admin route #40

Closed
Sacramentix opened this issue Nov 11, 2022 · 8 comments · Fixed by #46
Closed

add Netlify identity widget on admin route #40

Sacramentix opened this issue Nov 11, 2022 · 8 comments · Fixed by #46

Comments

@Sacramentix
Copy link

Currently if you don't set disableIdentityWidgetInjection to true then it add the Netlify Identity Widget to all page which can lead to unnecessary ressource being loaded in most case.

I would be nice to have a options to only inject it on admin routes.

@delucis
Copy link
Owner

delucis commented Nov 17, 2022

Should be feasible. Maybe we could go for something like:

NetlifyCMS({
  injectIdentityWidget: 'site' | 'admin-only' | false
})

This would default to 'site' and replace the current option for disabling the widget injection.

Would you be interested in making a PR?

@SeanMcP
Copy link

SeanMcP commented Dec 8, 2022

Is there ever an instance where you wouldn't want identity on the admin page? An alternative might be to keep the current behavior and just include it in the admin-dashboard.astro component.

@delucis
Copy link
Owner

delucis commented Dec 8, 2022

The contributor who added the feature confirmed they could log in successfully so I guess it can work in certain circumstances?

#33 (comment)

@SeanMcP
Copy link

SeanMcP commented Dec 8, 2022

Good to know! Being able to log in makes sense, but I wonder how you can invite users without the identity script running somewhere on the site to handle the invite_token.

I guess if you add the option, then it is up to the user to handle.

@delucis
Copy link
Owner

delucis commented Dec 9, 2022

Actually tested this myself and I think comments in #33 may have been misleading. Looks like: if you were already logged in before, logging in again without the identity widget works. But if you are trying to log in for the first time (e.g. with a different browser/device or incognito window), you do need the identity widget to make that work.

Given that, I think your proposal of always including on /admin makes sense until we hear otherwise @SeanMcP 👍

@SeanMcP
Copy link

SeanMcP commented Dec 10, 2022

Sounds good. It's a small change, but I would be happy to make it if that would be helpful.

@delucis
Copy link
Owner

delucis commented Dec 10, 2022

Absolutely — a contribution would be great!

@delucis
Copy link
Owner

delucis commented Dec 10, 2022

Thanks again for offering to make this change @SeanMcP — decided I needed it myself for a project I was working on so got it implemented and released in v0.3.5 🙌

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 a pull request may close this issue.

3 participants