appsmith icon indicating copy to clipboard operation
appsmith copied to clipboard

Published 20 hours ago • verified publisher icon appsmithorg

feat: update button states

Open tanvibhakta opened this issue 2 years ago • 31 comments

Description

This PR will deprecate existing Button.secondary styles, move all existing tertiary styles to secondary styles, and introduce a new Button.tertiary style.

Figma: https://www.figma.com/file/4aZWeRRAyIBX660EbIQhXn/DS-Upgrade-Exploration?node-id=1300%3A51079&t=WukGa4BEv3QsqRKc-0

Fixes https://github.com/appsmithorg/appsmith/issues/18052 Depends on https://github.com/appsmithorg/design-system/pull/255, https://github.com/appsmithorg/appsmith/pull/18231

Playground: https://design-system-olw3up1d3-get-appsmith.vercel.app/?path=/story/design-system-button--secondary https://design-system-olw3up1d3-get-appsmith.vercel.app/?path=/story/design-system-button--tertiary

Type of change

  • New feature (non-breaking change which adds functionality)

How Has This Been Tested?

  • tested in dev environment on storybook
  • Jest
  • cypress

Test Plan

Issues raised during DP testing

Checklist:

Dev activity

  • [ ] My code follows the style guidelines of this project
  • [ ] I have performed a self-review of my own code
  • [ ] I have commented my code, particularly in hard-to-understand areas
  • [ ] I have made corresponding changes to the documentation
  • [ ] My changes generate no new warnings
  • [ ] I have added tests that prove my fix is effective or that my feature works
  • [ ] New and existing unit tests pass locally with my changes
  • [ ] PR is being merged under a feature flag

QA activity:

  • [ ] Test plan has been approved by relevant developers
  • [ ] Test plan has been peer reviewed by QA
  • [ ] Cypress test cases have been added and approved by either SDET or manual QA
  • [ ] Organized project review call with relevant stakeholders after Round 1/2 of QA
  • [ ] Added Test Plan Approved label after reveiwing all Cypress test

tanvibhakta avatar Nov 22 '22 04:11 tanvibhakta

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

Name Status Preview Updated
appsmith ✅ Ready (Inspect) Visit Preview Dec 7, 2022 at 9:40AM (UTC)

vercel[bot] avatar Nov 22 '22 04:11 vercel[bot]

Unable to find test scripts. Please add necessary tests to the PR.

github-actions[bot] avatar Nov 22 '22 04:11 github-actions[bot]

/ok-to-test sha=4f5e55cc4383f90797f48d1286d6793103bbd192

tanvibhakta avatar Nov 22 '22 04:11 tanvibhakta

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3520332147. Workflow: Appsmith External Integration Test Workflow. Commit: 4f5e55cc4383f90797f48d1286d6793103bbd192. PR: 18335. Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-63465d4789020c7ac296d08d?pr=18335&runId=3520332147_1

github-actions[bot] avatar Nov 22 '22 04:11 github-actions[bot]

/ok-to-test sha=77e71bd7737c598c4aaa54c9deae7dbc55dab2cb

tanvibhakta avatar Nov 22 '22 05:11 tanvibhakta

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3520615311. Workflow: Appsmith External Integration Test Workflow. Commit: 77e71bd7737c598c4aaa54c9deae7dbc55dab2cb. PR: 18335. Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-63465d4789020c7ac296d08d?pr=18335&runId=3520615311_1

github-actions[bot] avatar Nov 22 '22 05:11 github-actions[bot]

/ok-to-test sha=10ea5aa528c078696b419c4fe231cf6b2c1947be

tanvibhakta avatar Nov 25 '22 16:11 tanvibhakta

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3549525628. Workflow: Appsmith External Integration Test Workflow. Commit: 10ea5aa528c078696b419c4fe231cf6b2c1947be. PR: 18335. Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-63465d4789020c7ac296d08d?pr=18335&runId=3549525628_1

github-actions[bot] avatar Nov 25 '22 16:11 github-actions[bot]

/ok-to-test sha=968e7e33e7c5e576adb4ffd50942b8b13193d7ee

tanvibhakta avatar Nov 28 '22 01:11 tanvibhakta

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3560909506. Workflow: Appsmith External Integration Test Workflow. Commit: 968e7e33e7c5e576adb4ffd50942b8b13193d7ee. PR: 18335. Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-63465d4789020c7ac296d08d?pr=18335&runId=3560909506_1

github-actions[bot] avatar Nov 28 '22 01:11 github-actions[bot]

@tanvibhakta is the DS PR for button states merged into this branch? I don't see the changes here in this PR. e.g. for Secondary Button the text colour does not match with the design in any of the sates. Also I am not able to trigger the focus state for both secondary & tertiary buttons using keyboard nav.

shadabbuchh avatar Nov 28 '22 07:11 shadabbuchh

e.g. for Secondary Button the text colour does not match with the design in any of the sates.

I see that the text color is black-700 on both the storybook and the dp. It looks all correct from my end. I'm not sure what you're not seeing, could you please double check? https://www.loom.com/share/3caf552e58d14a939e57f7eacfe1feb1

Also I am not able to trigger the focus state for both secondary & tertiary buttons using keyboard nav.

Unfortunately currently in our application we have a rule that disallows focus states across the board, and I cannot remove that because then it will cause focus states that haven't been calibrated to appear everywhere. You can test it by disallowing that rule within the css inspector if you'd like. We have added it now for future proofing.

tanvibhakta avatar Nov 29 '22 04:11 tanvibhakta

Unable to find test scripts. Please add necessary tests to the PR.

github-actions[bot] avatar Nov 29 '22 04:11 github-actions[bot]

I see that the text color is black-700 on both the storybook and the dp. It looks all correct from my end. I'm not sure what you're not seeing, could you please double check? https://www.loom.com/share/3caf552e58d14a939e57f7eacfe1feb1

@tanvibhakta I have marked the places where the design is not accurate in the below sheet. Also I have mentioned places where tertiary buttons are being used. https://docs.google.com/spreadsheets/d/1np7jQdiQa0nyryOBnNa927NkGDplG9M2gb7qnoZIIyM/edit?pli=1#gid=64982643&fvid=1110130271

shadabbuchh avatar Nov 29 '22 05:11 shadabbuchh

This happened because:

  1. I didn't realise some ads components were using tertiary button. Fixed.
  2. Buttons within Property Pane were using custom styles that overrid our defintions. Fixed.

tanvibhakta avatar Nov 30 '22 16:11 tanvibhakta

/ok-to-test sha=ca61b04de5ae19c5f6a7878527357f26f4f5d226

tanvibhakta avatar Nov 30 '22 16:11 tanvibhakta

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3585270969. Workflow: Appsmith External Integration Test Workflow. Commit: ca61b04de5ae19c5f6a7878527357f26f4f5d226. PR: 18335. Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-63465d4789020c7ac296d08d?pr=18335&runId=3585270969_1

github-actions[bot] avatar Nov 30 '22 16:11 github-actions[bot]

/ok-to-test sha=246e870

albinAppsmith avatar Dec 01 '22 07:12 albinAppsmith

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3590803784. Workflow: Appsmith External Integration Test Workflow. Commit: 246e870. PR: 18335. Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-63465d4789020c7ac296d08d?pr=18335&runId=3590803784_1

github-actions[bot] avatar Dec 01 '22 08:12 github-actions[bot]

/ok-to-test sha=e8a5034

albinAppsmith avatar Dec 02 '22 04:12 albinAppsmith

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3598871646. Workflow: Appsmith External Integration Test Workflow. Commit: e8a5034. PR: 18335. Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-63465d4789020c7ac296d08d?pr=18335&runId=3598871646_1

github-actions[bot] avatar Dec 02 '22 04:12 github-actions[bot]

All raised issues are fixed. Moving this issue to Done.

shadabbuchh avatar Dec 02 '22 12:12 shadabbuchh

:tada: All dependencies have been resolved !

dpulls[bot] avatar Dec 05 '22 02:12 dpulls[bot]

/ok-to-test sha=9d031aff4b330d4e3dc0dbe82893877267d97522

tanvibhakta avatar Dec 05 '22 05:12 tanvibhakta

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3617138212. Workflow: Appsmith External Integration Test Workflow. Commit: 9d031aff4b330d4e3dc0dbe82893877267d97522. PR: 18335. Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-63465d4789020c7ac296d08d?pr=18335&runId=3617138212_1

github-actions[bot] avatar Dec 05 '22 05:12 github-actions[bot]

/ok-to-test sha=e6863758912333f2fa9a7c8ccb5af72396377031

tanvibhakta avatar Dec 05 '22 18:12 tanvibhakta

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3622902581. Workflow: Appsmith External Integration Test Workflow. Commit: e6863758912333f2fa9a7c8ccb5af72396377031. PR: 18335. Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-63465d4789020c7ac296d08d?pr=18335&runId=3622902581_1

github-actions[bot] avatar Dec 05 '22 18:12 github-actions[bot]

/ok-to-test sha=e6863758912333f2fa9a7c8ccb5af72396377031

tanvibhakta avatar Dec 06 '22 02:12 tanvibhakta

Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3625803402. Workflow: Appsmith External Integration Test Workflow. Commit: e6863758912333f2fa9a7c8ccb5af72396377031. PR: 18335. Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-63465d4789020c7ac296d08d?pr=18335&runId=3625803402_1

github-actions[bot] avatar Dec 06 '22 02:12 github-actions[bot]

/ok-to-test sha=cdb3fca3740c990cc97f4c6917faf229bec059d7

tanvibhakta avatar Dec 06 '22 11:12 tanvibhakta