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

feat: account history component style and code revamp #128

Merged
merged 10 commits into from
Oct 4, 2023

Conversation

abdulhaseeb13mar
Copy link
Contributor

@abdulhaseeb13mar abdulhaseeb13mar commented Sep 25, 2023

fixes #120

i have opened this PR, for now it is not responsive for tab and mobile. will be working on it's responsiveness in the coming days

@netlify
Copy link

netlify bot commented Sep 25, 2023

Deploy Preview for chipper-sunburst-578cfe ready!

Name Link
🔨 Latest commit bdd0280
🔍 Latest deploy log https://app.netlify.com/sites/chipper-sunburst-578cfe/deploys/651d07feaa82ba0007e2be2b
😎 Deploy Preview https://deploy-preview-128--chipper-sunburst-578cfe.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@abdulhaseeb13mar abdulhaseeb13mar requested review from maelsar, EvgeniiaVak and EngineerCharlie and removed request for maelsar September 25, 2023 13:28
Copy link
Member

@EvgeniiaVak EvgeniiaVak left a comment

Choose a reason for hiding this comment

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

I tried to have an open order and the order count indicator is drawn on the second row (my window width is 1470px here):

Screenshot 2023-09-26 at 16 27 48

Is it possible to show them on the same row?

@abdulhaseeb13mar
Copy link
Contributor Author

I tried to have an open order and the order count indicator is drawn on the second row (my window width is 1470px here):
Is it possible to show them on the same row?

@EvgeniiaVak i have fixed it, you can check now

Copy link
Member

@EvgeniiaVak EvgeniiaVak left a comment

Choose a reason for hiding this comment

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

Border radii should probably be 0 for both the count indicator and the cancel button on hover to look consistent with the rest of the styling, which is quite rectangular. Strange that they are not absent by default on the button, we have it in the tailwind config for all the buttons.

The cancel button (when without hover) looks a little bit out of place - shifted to the right, 🤔 maybe moving the cancel button a little bit (e.g. use negative padding? what can go wrong? 😆 ). Or maybe remove the on hover at all, or only underline on hover? I vote for only underlining, with no bg changes.

Other differences from figma:

  1. all headers are uppercase
  2. cancel is red
  3. the selected tab has accent color
Screenshot 2023-09-26 at 18 55 05 Screenshot 2023-09-26 at 18 57 03

EvgeniiaVak
EvgeniiaVak previously approved these changes Oct 2, 2023
@EvgeniiaVak EvgeniiaVak mentioned this pull request Oct 3, 2023
@EvgeniiaVak EvgeniiaVak dismissed their stale review October 4, 2023 06:38

I committed in this branch also, someone else needs to review my latest commits.

Copy link
Member

@EvgeniiaVak EvgeniiaVak left a comment

Choose a reason for hiding this comment

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

@nguvictor reviewed (thanks), so approving this PR.

@EvgeniiaVak EvgeniiaVak merged commit c5ffdd8 into DeXter-on-Radix:main Oct 4, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Account History styling
3 participants