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: refactor add network to use card footer, closes leather-io/issue… #5849

Merged
merged 1 commit into from
Sep 20, 2024

Conversation

pete-watters
Copy link
Contributor

@pete-watters pete-watters commented Sep 6, 2024

Try out Leather build f928219Extension build, Test report, Storybook, Chromatic

This issues addresses a bug raised in https://github.com/leather-io/issues/issues/313. I've made some adjustments here to use the CardFooter so that it matches up with the previous Add a network sheet.

Area.mp4

@pete-watters pete-watters self-assigned this Sep 6, 2024
@pete-watters pete-watters requested a review from mica000 September 6, 2024 14:10
@mica000
Copy link

mica000 commented Sep 6, 2024

Everything looks good except the padding inside the page which Im not sure it's related to container. Ideally the padding on the extension pop up should be 24px left and right. Right now it looks like 32px or 40px?

image

@pete-watters pete-watters force-pushed the fix/313/add-network-ui branch from 8927f57 to 1d44909 Compare September 6, 2024 14:45
@pete-watters
Copy link
Contributor Author

Everything looks good except the padding inside the page which Im not sure it's related to container. Ideally the padding on the extension pop up should be 24px left and right. Right now it looks like 32px or 40px?

image

I updated it, thanks!
Screenshot 2024-09-06 at 15 46 02

@mica000
Copy link

mica000 commented Sep 6, 2024

LGTM

@pete-watters pete-watters force-pushed the fix/313/add-network-ui branch 2 times, most recently from b4a8d1e to a72705e Compare September 10, 2024 05:33
@markmhendrickson
Copy link
Collaborator

@pete-watters ready to merge?

@pete-watters
Copy link
Contributor Author

@pete-watters ready to merge?

It is but I can't merge it as we have a test failing in the CI (for all branches) that we need to investigate and fix.

It relates to #5854

@pete-watters pete-watters added this pull request to the merge queue Sep 20, 2024
Merged via the queue into dev with commit 68de602 Sep 20, 2024
30 checks passed
@pete-watters pete-watters deleted the fix/313/add-network-ui branch September 20, 2024 09:53
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