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.

Sign up for GitHub

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

Jump to bottom

feat: migrate to typescript #192

Merged
merged 20 commits into from
Jun 1, 2023
Merged

feat: migrate to typescript #192

merged 20 commits into from
Jun 1, 2023

Conversation

Majorfi
Copy link
Collaborator

@Majorfi Majorfi commented Mar 31, 2023

Work in Progress:

  • Migrate to typescript
  • Migrate to web-lib utils
  • Bump dependencies

@Majorfi Majorfi added the Improvement 🦄 Make something better label Mar 31, 2023
@Majorfi Majorfi self-assigned this Mar 31, 2023
@vercel
Copy link

vercel bot commented Mar 31, 2023 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
ape-tax ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 31, 2023 0:11am

@saltyfacu
Copy link
Collaborator

Great work here. I have some comments:

  • Some labels are missing: use production for example. You can check against ape.tax.
  • Text color is different.
    • Vault list: In ape.tax is darker which makes it easier, more comfortable, to read.
    • Inside a vault: in the top section both labels and data are dark, which makes it hard to identify different types of content. it should be like in the Wallet section.
    • Same in "Deploy your own vault"
  • In the top nav, the reverse ENS lookup is not working
  • When I click the wallet address to disconnect, it's stuck in "Loading Ex2" instead of showing the "not connected" message like in ape.tax

0xMirim
0xMirim reviewed May 12, 2023
View reviewed changes
Copy link
Contributor

@0xMirim 0xMirim left a comment

Choose a reason for hiding this comment

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

Thanks for the suggestion to review this. It definitely helped with understanding of where the core logic is and how project is organized! Time to build up on this!

@0xMirim @Itzabelli
refactor: Adjust styles to improve readability and Plausible setup (#198
d18db89 
)

* feat: setup plausible

* style: Adjust text color property

* style: Adjust text color

* style: Attempt to standardize colors for titles, text, and borders

* chore: add Issue / PR Templates

---------

Co-authored-by: Itzabelli <104786213+Itzabelli@users.noreply.github.com>
@0xMirim
chore: Correct name of templates folder (#199)
f917241 
Folder was accidently named "github" instead of ".github" meaning it wasn't usable in the way we originally intended
@0xMirim
style: adjust text styles to improve readability (#200)
642e86a 
Minor adjustments to more clearly show difference between labels and dyanamic values
@0xMirim
fix: Resolve console error by avoiding nested <a> tags (#202)
338b00c 
Error present because vault items contain links but tags (within these items) also link to other sites
@0xMirim
refactor: Change CG Symbol to always be lowercase (#204)
5725e26 
in vaults.json this attribute was sometimes upperase sometimes lowercase causing confusion when adding new entries
@Itzabelli
chore: Improve responsiveness of header (#208)
222b2d5 
Adjust media query of the title
@Itzabelli
refactor: Update react imports (#209)
9237e5c 
Adjust and remove unnecessary imports
@0xMirim
Copy link
Contributor

0xMirim commented May 29, 2023

Great work here. I have some comments:

* Some labels are missing: use production for example. You can check against ape.tax.

* Text color is different.
  
  * Vault list: In ape.tax is darker which makes it easier, more comfortable, to read.
  * Inside a vault: in the top section both labels and data are dark, which makes it hard to identify different types of content. it should be like in the Wallet section.
  * Same in "Deploy your own vault"

* In the top nav, the reverse ENS lookup is not working

* When I click the wallet address to disconnect, it's stuck in "Loading Ex2" instead of showing the "not connected" message like in ape.tax

All of these suggestions / concerns have been properly investigated / previously handled. Only thing preventing merge of this branch is the infinite load in the APR on each vault page and I should be able to take care of that shortly.

@0xMirim
fix: Resolve infinite load on vault apr (#210)
62c3ecf 
The suspense condition was causing loading to go on forever, slight adjustment corrected this
0xMirim
0xMirim approved these changes Jun 1, 2023
View reviewed changes
Copy link
Contributor

@0xMirim 0xMirim left a comment

Choose a reason for hiding this comment

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

lgtm

@0xMirim 0xMirim merged commit 38edd83 into master Jun 1, 2023
@0xMirim 0xMirim deleted the typescript branch June 1, 2023 13:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@0xMirim 0xMirim 0xMirim approved these changes

Assignees

@Majorfi Majorfi

Labels
Improvement 🦄 Make something better
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@Majorfi @saltyfacu @0xMirim @Itzabelli