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

Fix app bugs on Safari browser and Brave #420

Merged
merged 6 commits into from
Sep 15, 2023

Conversation

DemogorGod
Copy link
Contributor

list of things that i fixed

  • nav bar height on safari, some reason it takes more height on buttons, fixed now.
  • wallet select option would not close on outside click
  • amount input was only selectable on half of the input, now it's clickable on the whole thing
  • table export button height fixed
  • estimated staking rewards button height fixed
  • added a checkbox in the table headers to be able to select all or unselect all
  • table height was weird on safari so i set a min height for it
  • some text weight adjustments

@ccali11
Copy link
Contributor

ccali11 commented Sep 13, 2023

Looks like all of the things you mentioned are fixed when I run it locally. Nice work. You may also want take a look at the following:

  • to disable staking input until after user is signed in / wallet provider + address have been selected within in the staking component
  • + Add Operator button on safari (see screenshot below)
  • select address modal on safari (see screenshot below)
  • export button on Operator page once user is signed in (see screenshot below)
  • Add operator form sizes and spacing (see screenshot below)
  • In Safari (and I imagine other browsers as well), when I click on my address in the Register Operator modal, it closes the modal for some reason (let me know if you want me to share screen and show you)

image
image
image
image

@DemogorGod
Copy link
Contributor Author

Looks like all of the things you mentioned are fixed when I run it locally. Nice work. You may also want take a look at the following:

  • to disable staking input until after user is signed in / wallet provider + address have been selected within in the staking component
  • + Add Operator button on safari (see screenshot below)
  • select address modal on safari (see screenshot below)
  • export button on Operator page once user is signed in (see screenshot below)
  • Add operator form sizes and spacing (see screenshot below)
  • In Safari (and I imagine other browsers as well), when I click on my address in the Register Operator modal, it closes the modal for some reason (let me know if you want me to share screen and show you)

image
image
image
image

Ill take a look at these! Totally forgot about the operator page, I'll get em fixed here shortly

@DemogorGod
Copy link
Contributor Author

Looks like all of the things you mentioned are fixed when I run it locally. Nice work. You may also want take a look at the following:

  • to disable staking input until after user is signed in / wallet provider + address have been selected within in the staking component
  • + Add Operator button on safari (see screenshot below)
  • select address modal on safari (see screenshot below)
  • export button on Operator page once user is signed in (see screenshot below)
  • Add operator form sizes and spacing (see screenshot below)
  • In Safari (and I imagine other browsers as well), when I click on my address in the Register Operator modal, it closes the modal for some reason (let me know if you want me to share screen and show you)

image image image image

@ccali11
I left the first one unchecked because we want the user to be able to experience the component without having to connect wallet if they are new into the space.

@ccali11 the last one was due to the await getUserOperators() in Operator.vue
I added a comment there for you to move it to the composable and perhaps create an object under the user that includes all possible operators for each wallet they have connected. It's commented out for now due that bug existing when it runs.

@ccali11
Copy link
Contributor

ccali11 commented Sep 14, 2023

All sounds great. I will move the getUserOperators into the user.ts composable after I merge my existing PR; I might have already done it actually. 😅

Found another flexbox issue you might want to fix in this PR:
image

Very small thing to fix whenever:
image

@ccali11
Copy link
Contributor

ccali11 commented Sep 15, 2023

@DemogorGod - Nice fixes. One more fe bug I found is when you click Connect Wallet, you'll notice that the components shift to left (margin to right of Staking component increases) (or at least something visually noticeable happens). Want to take a look at that in this PR? Or do it separately?

@DemogorGod
Copy link
Contributor Author

@DemogorGod - Nice fixes. One more fe bug I found is when you click Connect Wallet, you'll notice that the components shift to left (margin to right of Staking component increases) (or at least something visually noticeable happens). Want to take a look at that in this PR? Or do it separately?

I don't see that bug happening on my end

@ccali11
Copy link
Contributor

ccali11 commented Sep 15, 2023

@DemogorGod - Nice fixes. One more fe bug I found is when you click Connect Wallet, you'll notice that the components shift to left (margin to right of Staking component increases) (or at least something visually noticeable happens). Want to take a look at that in this PR? Or do it separately?

I don't see that bug happening on my end

@hawyar saw this bug as well. I'll ping you to screenshare.

@ccali11
Copy link
Contributor

ccali11 commented Sep 15, 2023

Great changes (including fixing margin issue when clicking ConnectWallet)! Nicely done! I will approve so you can merge.

Copy link
Contributor

@ccali11 ccali11 left a comment

Choose a reason for hiding this comment

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

Already shared my review. Tested just now and approved contingent upon successful build. 👍

@DemogorGod DemogorGod merged commit 36535a7 into develop Sep 15, 2023
1 check passed
@ccali11 ccali11 deleted the enhancement/app-fe-tweaks branch September 15, 2023 15:26
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.

2 participants