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

convert dashboard-pages to tailwind #540

Merged
merged 17 commits into from
Feb 2, 2025

Conversation

kayceeDev
Copy link
Contributor

No description provided.

@whateverfw
Copy link
Collaborator

@kayceeDev sync with main please

@djeck1432 djeck1432 linked an issue Jan 30, 2025 that may be closed by this pull request
@kayceeDev
Copy link
Contributor Author

@kayceeDev sync with main please

I will definitely do that in my next push.

@kayceeDev kayceeDev force-pushed the migrate-dahsboard branch 2 times, most recently from 2bd4266 to 63c64f5 Compare January 30, 2025 13:30
frontend/src/pages/withdraw/Withdraw.jsx Outdated Show resolved Hide resolved
frontend/src/pages/dashboard/Dashboard.jsx Outdated Show resolved Hide resolved
frontend/src/pages/dashboard/Dashboard.jsx Outdated Show resolved Hide resolved
frontend/src/pages/position-history/PositionHistory.jsx Outdated Show resolved Hide resolved
frontend/src/pages/add-deposit/AddDeposit.jsx Outdated Show resolved Hide resolved
@whateverfw whateverfw marked this pull request as ready for review January 30, 2025 21:21
@kayceeDev
Copy link
Contributor Author

@whateverfw The main is also broken. but I will try and fix them.

@whateverfw
Copy link
Collaborator

whateverfw commented Jan 31, 2025

@whateverfw The main is also broken. but I will try and fix them.

what main? do you mean that main branch also has issue on this pages?

It can be the case, because we are in the process of tailwind migration, so maybe someone somehow broke those pages. That's basically why we have this ticket to fix exactly those pages and another ones are fixed by another devs.

@kayceeDev
Copy link
Contributor Author

Yes, I will definitely fix them. In the mobile, I see a drop down menu popping out. I think the active state changes to true on mobile..

@whateverfw
Copy link
Collaborator

Yes, I will definitely fix them. In the mobile, I see a drop down menu popping out. I think the active state changes to true on mobile..

if that's blocking you and it is not a quick fix, just put some false state for now and concentrate on your pages, we'll fix it separately

@kayceeDev
Copy link
Contributor Author

Alright. Thanks

@kayceeDev
Copy link
Contributor Author

So the broken you are referring to for mobile and desktop, is it during the break points?

@whateverfw
Copy link
Collaborator

@kayceeDev I just attached all the screenshot, in general on few pages on desktop only cards styles are broken, however position history page has broken both desktop in mobile

and in general each page's mobile is not correct, as you can see on screenshots. I don't see any breakpoints for responsiveness in code

@kayceeDev
Copy link
Contributor Author

I will fix it. Thank you for the screenshots. I didn't clearly see the mobile completely cos of the overlay that comes up whenever you go to mobile.

Copy link
Collaborator

@whateverfw whateverfw left a comment

Choose a reason for hiding this comment

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

I see that mobile styles are not yet ideal, but better, pls take a look carefully and fix.

Copy link
Collaborator

Choose a reason for hiding this comment

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

please revert changes on in this file

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright. I will. Remove it from my commit

@kayceeDev kayceeDev force-pushed the migrate-dahsboard branch 2 times, most recently from 87686c1 to 1aa3ff4 Compare February 1, 2025 05:08
@kayceeDev
Copy link
Contributor Author

@whateverfw all fixes has been resolved and everything looks good on all devices now

Copy link
Collaborator

@whateverfw whateverfw left a comment

Choose a reason for hiding this comment

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

Please make sure to go through figma and make sure those pages looks like that.

Copy link
Collaborator

Choose a reason for hiding this comment

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

image

Copy link
Collaborator

Choose a reason for hiding this comment

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

still appears like that @kayceeDev

Copy link
Collaborator

Choose a reason for hiding this comment

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

take a look how mobile cards positioned in figma, also the main card padding slightly different and not all values are displayed
image

Copy link
Collaborator

Choose a reason for hiding this comment

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

image
clearly doesn't look like in figma (cards)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Collaborator

Choose a reason for hiding this comment

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

image

please seed this table with data (there is a command in README.md) and make sure it look good on both desktop and mobile

Copy link
Collaborator

Choose a reason for hiding this comment

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

still do not match figma desktop/mobile

Copy link
Collaborator

Choose a reason for hiding this comment

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

@kayceeDev
Copy link
Contributor Author

Some of the components on mobile has extra buttons
For example the deposit on mobile have a cancel button
Am I expected to add it in this migration or just make the existing components be as they are in the figma

@kayceeDev
Copy link
Contributor Author

Please clarify. Because some of the differences I see on mobile is the cards I turned to column instead of rows

1 similar comment
@kayceeDev
Copy link
Contributor Author

Please clarify. Because some of the differences I see on mobile is the cards I turned to column instead of rows

@whateverfw
Copy link
Collaborator

Some of the components on mobile has extra buttons For example the deposit on mobile have a cancel button Am I expected to add it in this migration or just make the existing components be as they are in the figma

make existing, if it didn't exist before - do not add

@whateverfw
Copy link
Collaborator

Please clarify. Because some of the differences I see on mobile is the cards I turned to column instead of rows

yeah, so, that was exactly one of the points, which aren't like in Figma

Copy link
Collaborator

@whateverfw whateverfw left a comment

Choose a reason for hiding this comment

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

still various broken styles or different looks from figma/previous version.

Copy link
Collaborator

@whateverfw whateverfw left a comment

Choose a reason for hiding this comment

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

@kayceeDev
I see that direction for fixes of position history is incorrect, please follow the figma, we had 3 columns and then action button that triggers a modal with the rest of the fields...
image

@kayceeDev kayceeDev force-pushed the migrate-dahsboard branch 2 times, most recently from ee56bd7 to a1ad7ea Compare February 2, 2025 12:04
@kayceeDev
Copy link
Contributor Author

@whateverfw fixed

@kayceeDev kayceeDev force-pushed the migrate-dahsboard branch 2 times, most recently from 18aa6a5 to 8254b9e Compare February 2, 2025 12:09
Copy link
Owner

@djeck1432 djeck1432 left a comment

Choose a reason for hiding this comment

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

Looks good to me

@djeck1432 djeck1432 merged commit 6e4e443 into djeck1432:main Feb 2, 2025
1 check passed
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.

[Frontend] Migrate Dashboard Pages
3 participants