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

chore(tx-builder-theme): Remove safe-react components and theme from the tx-builder application #823

Conversation

clovisdasilvaneto
Copy link
Contributor

@clovisdasilvaneto clovisdasilvaneto commented Sep 25, 2024

Current we're using an outdated theme in the transaction builder application. In order to have the right theme introduced on it, we need to first adjust the internal theme structure which we use in our components, and since the https://github.com/5afe/safe-react-components repo is achieved me and @katspaugh decided to move the components to the tx-builder/components folder and change the internal theme structure that the components are using.

What it solves

Resolves #820

How this PR fixes it

It uses the moved safe-react-components in the tx-builder screens

How to test it

Screenshot 2024-09-25 at 11 53 02

Copy link

CLA Assistant Lite bot:
Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.


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


You can retrigger this bot by commenting recheck in this Pull Request

@clovisdasilvaneto clovisdasilvaneto changed the title Chore/remove safe react components chore(tx-builder-theme): remove safe react components Sep 25, 2024
@clovisdasilvaneto clovisdasilvaneto changed the title chore(tx-builder-theme): remove safe react components chore(tx-builder-theme): use the tx-builder components Sep 25, 2024
@clovisdasilvaneto clovisdasilvaneto changed the title chore(tx-builder-theme): use the tx-builder components chore(tx-builder-theme): Remove safe-react-components from the tx-builder application Sep 25, 2024
@clovisdasilvaneto clovisdasilvaneto changed the title chore(tx-builder-theme): Remove safe-react-components from the tx-builder application chore(tx-builder-theme): Remove safe-react components and theme from the tx-builder application Sep 25, 2024
Comment on lines +2932 to +2935
"@safe-global/safe-gateway-typescript-sdk@^3.19.0":
version "3.22.2"
resolved "https://registry.yarnpkg.com/@safe-global/safe-gateway-typescript-sdk/-/safe-gateway-typescript-sdk-3.22.2.tgz#d4ff9972e58f9344fc95f8d41b2ec6517baa8e79"
integrity sha512-Y0yAxRaB98LFp2Dm+ACZqBSdAmI3FlpH/LjxOZ94g/ouuDJecSq0iR26XZ5QDuEL8Rf+L4jBJaoDC08CD0KkJw==
Copy link
Member

Choose a reason for hiding this comment

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

Strange that this appeared. Is it due to yarn install?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes

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.

Code looks good!

@clovisdasilvaneto clovisdasilvaneto force-pushed the chore/move-react-components-to-tx-builder branch from 248e46b to f0a67ea Compare September 27, 2024 10:51
@clovisdasilvaneto clovisdasilvaneto self-assigned this Oct 1, 2024
@francovenica
Copy link

Question. I see in the designs that the form should be on the full width of the view port. Is this only for small viewports?

This is how I currently see it
image

This is from the design:
image

This is the only difference I see, so if the width of the form is not part of the changes then the ticket is good to go

@clovisdasilvaneto
Copy link
Contributor Author

clovisdasilvaneto commented Oct 2, 2024

Hi @francovenica, yes the form width is not part of the change, however I think I can make it looks a bit better.

@francovenica
Copy link

Ok, the ticket how it is can be passed to "ready to merge". Let me know if you can try that change you suggested here or if you will create a new PR for it. Thanks

Base automatically changed from chore/move-react-components-to-tx-builder to chore/normalize-tx-builder-theme October 4, 2024 07:59
@clovisdasilvaneto clovisdasilvaneto merged commit e18e89a into chore/normalize-tx-builder-theme Oct 4, 2024
6 of 8 checks passed
@clovisdasilvaneto clovisdasilvaneto deleted the chore/remove-safe-react-components branch October 4, 2024 08:01
@github-actions github-actions bot locked and limited conversation to collaborators Oct 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants