Skip to content

Conversation

@bunsenstraat
Copy link
Collaborator

@bunsenstraat bunsenstraat commented Sep 2, 2025

  • removed loading state because there's no real way to be sure it's loading because it's handled by popup. and we don't want to disable the button in such a situation.
  • refactor login actions so they can be called from the plugin listener
  • refactor button to respond fully to app context
  • removed all login logic from the button
  • removed the extra button that was there in success state, it's now one button

@netlify
Copy link

netlify bot commented Sep 2, 2025

Deploy Preview for remixproject ready!

Name Link
🔨 Latest commit 38eae64
🔍 Latest deploy log https://app.netlify.com/projects/remixproject/deploys/68bf0442ea6a6c0008189b60
😎 Deploy Preview https://deploy-preview-6331--remixproject.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@joeizang
Copy link
Collaborator

joeizang commented Sep 3, 2025

  • The login button looks like its disabled. See the image below
image
  • Also the background of the button should not be this color. The previous color seems a better fit.

Everything else looks good to me.

Copy link
Collaborator

@joeizang joeizang left a comment

Choose a reason for hiding this comment

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

Left some comments

Copy link
Collaborator

@joeizang joeizang left a comment

Choose a reason for hiding this comment

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

The code changes are solid. I will approve when e2e are green.

@Aniket-Engg Aniket-Engg merged commit ba45e4c into master Sep 8, 2025
32 checks passed
@Aniket-Engg Aniket-Engg deleted the loginbutton branch September 8, 2025 16:44
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.

4 participants