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: Spending limits: Table header is displayed when no records #1703 #1852

Merged
merged 1 commit into from
Apr 14, 2023

Conversation

deepaksing
Copy link
Contributor

@deepaksing deepaksing commented Apr 7, 2023

What it solves

This solves the issue where Table header is displayed even when there are no records

Resolves #1703
I have added a conditonal statement which renders table only when data is avilable.

How this PR fixes it

table is displayed only when it's not empty

How to test it

Go to the spending limits page in the safe without the spending limits module enabled or to the Base network ( no support for spending limits). Now the table would not be rendered if there is no data in table.

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) 🧑‍💻

@github-actions
Copy link

github-actions bot commented Apr 7, 2023

CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅

@deepaksing
Copy link
Contributor Author

I have read the CLA Document and I hereby sign the CLA

Copy link
Member

@katspaugh katspaugh left a comment

Choose a reason for hiding this comment

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

Nice work, thank you! ❤️

@deepaksing
Copy link
Contributor Author

Hi, @katspaugh do I need to make any changes from my side for the failed checks?

@katspaugh
Copy link
Member

No, it’s all good, they fail because of missing permissions. We’re just waiting for our QA now.

@deepaksing
Copy link
Contributor Author

Ok, Thank you for informing.

@francovenica
Copy link
Contributor

Looks good.

In this repo I'm not able to execute tx so I can't create a spending limit to see if they table header pops up again. So I'll re check it when merged into dev

@iamacook iamacook merged commit 5a8140d into safe-global:dev Apr 14, 2023
@gitpoap-bot
Copy link

gitpoap-bot bot commented Apr 14, 2023

Congrats, your important contribution to this open-source project has earned you a GitPOAP!

GitPOAP: 2023 Safe Contributor:

GitPOAP: 2023 Safe Contributor GitPOAP Badge

Head to gitpoap.io & connect your GitHub account to mint!

Learn more about GitPOAPs here.

@github-actions github-actions bot locked and limited conversation to collaborators Apr 14, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Spending limits: Table header is displayed when no records
5 participants