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

Button css changes #10066

Merged
merged 7 commits into from
May 3, 2024
Merged

Button css changes #10066

merged 7 commits into from
May 3, 2024

Conversation

prachi00
Copy link
Member

Thank you for your contribution to the KodaDot - One Stop Shop for Polkadot NFTs.

👇 __ Let's make a quick check before the contribution.

PR Type

  • Bugfix
  • Feature
  • Refactoring

Needs QA check

  • @kodadot/qa-guild please review

Context

Did your issue had any of the "$" label on it?

Screenshot 📸

  • [] My fix has changed UI

Copilot Summary

@prachi00 prachi00 requested a review from a team as a code owner April 12, 2024 19:34
@prachi00 prachi00 requested review from daiagi and hassnian and removed request for a team April 12, 2024 19:34
@kodabot
Copy link
Collaborator

kodabot commented Apr 12, 2024

WARNING @prachi00 PR for issue #10018 which isn't assigned to you. Please be warned that this PR may get rejected if there's another assignee for issue #10018

Copy link

netlify bot commented Apr 12, 2024

Deploy Preview for koda-canary ready!

Name Link
🔨 Latest commit 70f8273
🔍 Latest deploy log https://app.netlify.com/sites/koda-canary/deploys/6633de54faf1420008c23a97
😎 Deploy Preview https://deploy-preview-10066--koda-canary.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.

@exezbcz
Copy link
Member

exezbcz commented Apr 13, 2024

and other buttons?

@prachi00
Copy link
Member Author

and other buttons?

which ones?

@prachi00
Copy link
Member Author

and other buttons?

which ones?

@exezbcz which ones? I changed the height, added focus, active states

@exezbcz
Copy link
Member

exezbcz commented Apr 16, 2024

  • regular button now has a height of 40px instead of 36. Will work better as a touchpoint

image

@prachi00
Copy link
Member Author

  • regular button now has a height of 40px instead of 36. Will work better as a touchpoint

image

I changed the common button component, is there any button that hasnt changed?

@prachi00
Copy link
Member Author

  • regular button now has a height of 40px instead of 36. Will work better as a touchpoint

image

I changed the common button component, is there any button that hasnt changed?

@exezbcz

@exezbcz
Copy link
Member

exezbcz commented Apr 19, 2024

the focus would be ideal if it would not scale the button and thus move all the elements in the UI
image

gallery item in share
image

  • perfect square please
  • 40x40

otherwise looking good!

could you please make sure there are no duplicates in styles, etc, so that there is not a mess in the styles and easily usable?

@prury prury changed the title #10018 btn changes Button css changes Apr 22, 2024
@prachi00
Copy link
Member Author

the focus would be ideal if it would not scale the button and thus move all the elements in the UI image

gallery item in share image

  • perfect square please
  • 40x40

otherwise looking good!

could you please make sure there are no duplicates in styles, etc, so that there is not a mess in the styles and easily usable?

@exezbcz done

@prury prury added the S-changes-requested-🤞 PR is almost good to go, just some fine tunning label Apr 23, 2024
Copy link
Contributor

@daiagi daiagi left a comment

Choose a reason for hiding this comment

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

this has been standing around way too long for a small change

@hassnian the story book is probably outdated and quite unused

I've looked around through the app, all buttons look ok, just a bit taller

however, the NeoButtonVariants are outdated and don't match current figma variants, let's change that

Copy link
Contributor

@hassnian hassnian left a comment

Choose a reason for hiding this comment

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

this has been standing around way too long for a small change

yes

I've looked around through the app, all buttons look ok, just a bit taller

tested and they look good , variants that have a different height already override the height inside the class.

@prachi00 let's continue here #10163

@prachi00
Copy link
Member Author

prachi00 commented May 2, 2024

can we merge this now?

Copy link

codeclimate bot commented May 2, 2024

Code Climate has analyzed commit 70f8273 and detected 0 issues on this pull request.

View more on Code Climate.

Copy link

sonarcloud bot commented May 2, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.5% Duplication on New Code

See analysis details on SonarCloud

@hassnian
Copy link
Contributor

hassnian commented May 3, 2024

@prury can we test and merge this one?

thanks

@prury
Copy link
Member

prury commented May 3, 2024

I've looked around through the app, all buttons look ok, just a bit taller

indeed

@prury prury added this pull request to the merge queue May 3, 2024
Merged via the queue into kodadot:main with commit c97ec5c May 3, 2024
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-changes-requested-🤞 PR is almost good to go, just some fine tunning
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Consistent button components
6 participants