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: disabled "Enable all" button height #2574

Merged
merged 2 commits into from
Oct 2, 2023
Merged

Conversation

iamacook
Copy link
Member

@iamacook iamacook commented Oct 2, 2023

What it solves

Resolves style inconsistency for "Enable all" button height when disabled.

How this PR fixes it

The CheckWallet component now wraps the button container element instead of just the "Enable all" button.

How to test it

Note: this seems to happen only in Chrome.

  1. Ensure no wallet is connect.
  2. Ensure the Safe has not previously dismissed the push notification banner.
  3. Open the Safe.
  4. Observe the "Enable all" button (disabled) having the same height as the "Customize" button in the notification banner.

Screenshots

image

Checklist

  • I've tested the branch on mobile 📱
  • I've documented how it affects the analytics (if at all) 📊
  • I've written a unit/e2e test for it (if applicable) 🧑‍💻

@iamacook iamacook self-assigned this Oct 2, 2023
@github-actions
Copy link

github-actions bot commented Oct 2, 2023

Branch preview

✅ Deploy successful!

https://notification_enable_button--walletweb.review-wallet-web.5afe.dev

@github-actions
Copy link

github-actions bot commented Oct 2, 2023

ESLint Summary View Full Report

Annotations are provided inline on the Files Changed tab. You can also see all annotations that were generated on the annotations page.

Type Occurrences Fixable
Errors 0 0
Warnings 0 0
Ignored 0 N/A
  • Result: ✅ success
  • Annotations: 0 total

Report generated by eslint-plus-action

@francovenica
Copy link
Contributor

Is barely noticeable but ok:

This is how it looks in chrome in firefox:
image
image

The button enabled still looks good:
image

@iamacook iamacook merged commit 5d5466d into dev Oct 2, 2023
9 checks passed
@iamacook iamacook deleted the notification-enable-button branch October 2, 2023 12:49
@github-actions github-actions bot locked and limited conversation to collaborators Oct 2, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

3 participants