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

feat: arkconnect navbar button #799

Merged
merged 40 commits into from
Mar 26, 2024

Conversation

alexbarnsley
Copy link
Member

@alexbarnsley alexbarnsley commented Mar 19, 2024

Summary

https://app.clickup.com/t/86drwqqf8

requires ArkEcosystem/laravel-foundation#613

Todo:

  • Mobile state - awaiting designs
  • .env option to disable & add a test for it

To test:

  • add ARKCONNECT_ENABLED=true to .env
  • make sure you're connecting to a https instance of explorer. For valet, you can use valet secure to force https - use valet unsecure to change it back to http

Checklist

  • I checked my UI changes against the design and there are no notable differences
  • I checked my UI changes for any responsiveness issues
  • I checked my UI changes in light AND dark mode
  • I checked my (code) changes for obvious issues, debug statements and commented code
  • I opened a corresponding card on Clickup for any remaining TODOs in my code
  • I added a short description on how to test this PR (if necessary)
  • Documentation (if necessary)
  • Tests (if necessary)
  • Ready to be merged

@alexbarnsley alexbarnsley marked this pull request as ready for review March 20, 2024 10:46
@alexbarnsley alexbarnsley changed the title [wip] feat: arkconnect navbar button feat: arkconnect navbar button Mar 20, 2024
@alexbarnsley alexbarnsley changed the base branch from develop to feat/arkconnect March 20, 2024 10:52
@alexbarnsley alexbarnsley marked this pull request as draft March 20, 2024 10:53
@alexbarnsley alexbarnsley marked this pull request as ready for review March 20, 2024 15:18
Copy link
Contributor

@alfonsobries alfonsobries left a comment

Choose a reason for hiding this comment

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

This doesnt seem right
image

image

Copy link
Contributor

@alfonsobries alfonsobries left a comment

Choose a reason for hiding this comment

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

Other than the mode-color the design is not applied properly, design:

image

Copy link
Contributor

@alfonsobries alfonsobries left a comment

Choose a reason for hiding this comment

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

Finally the hasExtension() condition doesnt seem to work for me (i removed that condition to test this PR)

Tried to install the latest version of the arkconnect extension on chrome+mac and window.arkconnect is undefined, same happens if I run the develop version of the plugin, not sure if I missing something

update:wasnt using https

@alexbarnsley alexbarnsley marked this pull request as draft March 20, 2024 22:49
@alexbarnsley alexbarnsley marked this pull request as ready for review March 21, 2024 20:57
Copy link
Contributor

@alfonsobries alfonsobries left a comment

Choose a reason for hiding this comment

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

design is good now, but now the button is always disabled

image

Probably because of this line is never setting isLoading to false

https://github.com/ArdentHQ/arkscan/pull/799/files#diff-107b51fb3ed89d5d781341dc4e51b926a020477c468cf86edc0b9d24603e17d7R25-R27

Also in my case I have arkconnect, maybe when that function runs the window.arkconnect is not set yet

@alfonsobries alfonsobries marked this pull request as draft March 22, 2024 00:14
@alexbarnsley alexbarnsley marked this pull request as ready for review March 25, 2024 17:50
@alfonsobries
Copy link
Contributor

working now @alexbarnsley

@ItsANameToo ItsANameToo merged commit b78097f into feat/arkconnect Mar 26, 2024
14 checks passed
@ItsANameToo ItsANameToo deleted the feat/arkconnect-navbar-button branch March 26, 2024 08:57
@alfonsobries alfonsobries restored the feat/arkconnect-navbar-button branch March 26, 2024 21:42
@ItsANameToo ItsANameToo added this to the 2.25.0 milestone Apr 17, 2024
@ItsANameToo ItsANameToo deleted the feat/arkconnect-navbar-button branch December 11, 2024 10:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants