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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

(feat): custom signin page #6016

Conversation

NiazMorshed2007
Copy link
Contributor

@NiazMorshed2007 NiazMorshed2007 commented Apr 12, 2023

This feature closes #5856 by adding custom login page.

What I included?

1. Configuration part

I added some configuration in the [...nextauth].js file to allow creating custom page in auth/signin. Changes I added 馃憞

const authOptions = {
// ...
  pages: {
    signIn: "/auth/signin",
  }
};

2. Design and connecting the login functionality part

Kept the design simple but I'm always open to redesign if you wish!

Can I see a screenshot?

Sure! Here is how it's currently look like:


image

How did I managed the login funtionality?

I followed the best practices from NextAuth documentation to implement this. Here is a little brief about it 馃憞:

  1. First of all I imported these necessary functions and modules
import { getServerSession } from "next-auth/next"
import { getProviders, signIn } from "next-auth/react"
import { authOptions } from "pages/api/auth/[...nextauth]"
  1. And then with getServerSideProps() function I checked if the user is already logged in or not. If the user is logged I sent them to / page and if not I pushed the providers in the props of the page
export async function getServerSideProps(context) {
    const session = await getServerSession(context.req, context.res, authOptions);

    // if the user is logged in redirect, (note: don't redirect to the same page otherwise it'll be in a infinite loop)
    if (session) {
        return { redirect: { destination: "/" } };
    }

    const providers = await getProviders();

    return {
        props: { providers: providers ?? [] },
    }
}
  1. And then I grabbed the providers from the props and turned providers into an array using Object.values() and mapped over to display different auth providers. Notice the handleProviderIcon function here, it will check the provider name and render icon accordingly.
 {Object.values(providers).map((provider) => (
                    <button
                        key={provider.name}
                        onClick={() => signIn(provider.id)}
                        className="group relative transition-all flex gap-4 items-center w-full justify-center rounded-md bg-primary-medium p-3 text-sm font-medium text-white hover:bg-primary-medium/60 outline-none"
                    >
                        {handleProviderIcon(provider.name)} Continue with {provider.name}
                    </button>
 ))}

The handleProviderIcon function:

    const handleProviderIcon = (provider_name) => {
        if (provider_name === "GitHub") {
            return <BsGithub className="text-xl" />
        }
    }

That's is the wrap.
I look forward for reviews.
Thanks!馃檶

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

It's great having you contribute to this project

Welcome to the community 馃

If you would like to continue contributing to open source and would like to do it with an awesome inclusive community, you should join our Discord chat and our GitHub Organisation - we help and encourage each other to contribute to open source little and often 馃 . Any questions let us know.

@github-actions github-actions bot added the large Pull request with more than 30 changed lines label Apr 12, 2023
@ChinmayMhatre
Copy link
Member

ChinmayMhatre commented Apr 12, 2023

A couple of suggestion from my end :

  • Add tailwind classes for both dark mode and light mode by using the :dark variant ( let me know if you need any help with this )
  • Take into consideration the contrast ratio between the background and foreground elements. (when using bg-primary-high use text-primary-low or text-primary-low-medium) (use this website to test the contrast ratio: https://contrast-ratio.com)
  • Use the colours from the tailwind css config to keep consistency

iamthecloverly
iamthecloverly previously approved these changes Apr 12, 2023
@NiazMorshed2007
Copy link
Contributor Author

A couple of suggestion from my end

Thanks @ChinmayMhatre for the suggestion, I'll work on this when I get free, also I may reach out to you on EddieHub discrod for help :)

@SaraJaoude SaraJaoude added the issue linked Pull Request has issue linked label Apr 13, 2023
@NiazMorshed2007
Copy link
Contributor Author

@ChinmayMhatre I've done some tweaking. Can you review it?

@ChinmayMhatre
Copy link
Member

@ChinmayMhatre I've done some tweaking. Can you review it?

I'll take a look tommorow!

@ChinmayMhatre
Copy link
Member

@NiazMorshed2007 please add screenshots of the page in light mode and dark mode

@NiazMorshed2007
Copy link
Contributor Author

@ChinmayMhatre Sure thing!

The light version:
image

The dark version:
image

@ChinmayMhatre
Copy link
Member

Ui looks good to me. Maybe @eddiejaoude can take a look for any more optimizations.

pages/auth/signin.js Outdated Show resolved Hide resolved
pages/auth/signin.js Outdated Show resolved Hide resolved
return (
<BlankLayout>
{page}
</BlankLayout>
Copy link
Member

Choose a reason for hiding this comment

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

The formatting doesn't look quite right, can you check you are using our Prettier config in VScode

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've done the other requested changes. But before I commit I've the issue with prettier still. Is the configuration for prettier is the .prettierrc file? I got it empty

image

Copy link
Member

Choose a reason for hiding this comment

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

yep we use the default, that is fine, the plugin will work on save usually

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, but it doesn't fix the formatting. I've pinged on discord about this issue.

Copy link
Member

@eddiejaoude eddiejaoude left a comment

Choose a reason for hiding this comment

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

Looks great 馃憤 some inline comments

export default function SignIn({ providers }) {

const handleProviderIcon = (provider_name) => {
if (provider_name === "GitHub") {
Copy link
Member

Choose a reason for hiding this comment

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

A switch would be better here and then add a default at the end for a fallback login icon

Copy link
Member

Choose a reason for hiding this comment

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

But for now maybe just add a return out side of the if with a default key/secure icon

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You mean like this?

const handleProviderIcon = (provider_name) => {
        if (provider_name === "GitHub") {
            return <BsGithub className="text-xl" />
        }

        return <LockIcon />
    }

Copy link
Member

Choose a reason for hiding this comment

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

yes 馃憤 , but you might want to add the same class to it also

@eddiejaoude eddiejaoude changed the base branch from main to fix-login-page May 3, 2023 01:20
Copy link
Member

@eddiejaoude eddiejaoude left a comment

Choose a reason for hiding this comment

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

Thank you 馃憤

@eddiejaoude eddiejaoude merged commit f64f662 into EddieHubCommunity:fix-login-page May 3, 2023
9 checks passed
@eddiejaoude eddiejaoude mentioned this pull request May 3, 2023
6 tasks
eddiejaoude added a commit that referenced this pull request May 4, 2023
* feat: custom signin page (#6016)

* (feat): login page re-designed

* proposed-changes

* (update): custom login page

---------

Co-authored-by: Amanda <97615019+amandamartin-dev@users.noreply.github.com>
Co-authored-by: Eddie Jaoude <eddie@jaoudestudios.com>

* feat: custom login page

* fix: sub title text

---------

Co-authored-by: Niaz Morshed <77217706+NiazMorshed2007@users.noreply.github.com>
Co-authored-by: Amanda <97615019+amandamartin-dev@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
issue linked Pull Request has issue linked large Pull request with more than 30 changed lines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] Improve login page design
7 participants