appsmith
appsmith copied to clipboard
appsmithorg
chore: use link from ds
Description
Replaces custom link components with minimally styled ones from the design-system in the user auth flow. Fixes #https://github.com/appsmithorg/appsmith/issues/17739
In this PR, I have removed a global rule which says "no outlines". This allows our current design system to display the outlines, and makes it clear what other links we need to change/inputs we need to design etc, by the end of this effort.
Design File: https://www.figma.com/file/4aZWeRRAyIBX660EbIQhXn/DS-Upgrade-Exploration?node-id=766%3A39347
Depends on https://github.com/appsmithorg/design-system/pull/242, https://github.com/appsmithorg/appsmith/pull/18231
Type of change
- New feature (non-breaking change which adds functionality)
How Has This Been Tested?
Locally, and on the dp
Flows to test
All the links on the UserAuth flow -
- forgot password
- T&C + privacy policy
- Sign Up
- Back to Sign Up
Checklist:
- [ ] 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
The latest updates on your projects. Learn more about Vercel for Git ↗︎
| Name | Status | Preview | Updated |
|---|---|---|---|
| appsmith | ✅ Ready (Inspect) | Visit Preview | Dec 6, 2022 at 3:17AM (UTC) |
![vercel[bot] avatar](https://avatars.githubusercontent.com/in/8329?v=4 "Published by a pub.dev verified publisher"){kind=small}
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3286272168.
Workflow: Appsmith External Integration Test Workflow.
Commit: 0e9d091b4e510c43dba8f9686b1c86d9c916a952.
PR: 17740.
Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-63465d4789020c7ac296d08d?pr=17740&runId=3286272168_1
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
/perf-test ref=rhito/feat-create-run-meta-before-test-starts-compare-lt-st-for-prev-runs
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3286341939.
Workflow: Appsmith External Integration Test Workflow.
Commit: ``.
PR: 17740.
Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-63465d4789020c7ac296d08d?pr=17740&runId=3286341939_1
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3303222345.
Workflow: Appsmith External Integration Test Workflow.
Commit: 0e9d091b4e510c43dba8f9686b1c86d9c916a952.
PR: 17740.
Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-63465d4789020c7ac296d08d?pr=17740&runId=3303222345_1
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3309596993.
Workflow: Appsmith External Integration Test Workflow.
Commit: 0e9d091b4e510c43dba8f9686b1c86d9c916a952.
PR: 17740.
Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-63465d4789020c7ac296d08d?pr=17740&runId=3309596993_1
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3311264305.
Workflow: Appsmith External Integration Test Workflow.
Commit: 0e9d091b4e510c43dba8f9686b1c86d9c916a952.
PR: 17740.
Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-63465d4789020c7ac296d08d?pr=17740&runId=3311264305_1
Issues that were identified:
- The font colour & size for
Forgot Password,Privacy Policy&Terms & Conditionslinks is different from release/prod. -
Sign In/Sign Uptext is bold and looks interact-able even when email/password are not entered.
-
The underline is not continuous, it breaks for letters such as
g,p,y.
-
Do we want the Pressed state to persist when user is redirected to a different tab? e.g. when we click on
Privacy Policy. https://www.loom.com/share/f23f4de4a2ff42628a89a0d19ee08fd2
The font color & size for Forgot Password, Privacy Policy & Terms & Conditions links is different from release/prod.
The goal is for these to follow the guidelines as set in the design files, not in release/prod. Could you please check if the guidelines are followed from the design?
- color, hover, pressed behavior from design
- font sizes from prod
Sign In/Sign Up text is bold and looks interact-able even when email/password are not entered.
My code doesn't change this. In fact, while this isn't visible on release or prod environments, this is visible even on the release branch on local. I'm not sure what's happening here.
The underline is not continuous, it breaks for letters such as g, p, y.
This is default browser behavior - typically we want underline to break on descenders, but let me confirm with @vasanth-appsmith.
Do we want the Pressed state to persist
This is also default browser behaviour to allow users to be able to resume context when they come back to a page, but I will confirm with design.
The goal is for these to follow the guidelines as set in the design files, not in release/prod. Could you please check if the guidelines are followed from the design?
- color, hover, pressed behavior from design
- font sizes from prod
Font size is different from prod
This is default browser behavior - typically we want underline to break on descenders, but let me confirm with @vasanth-appsmith.
As of now this behaviour is not happening in release/prod. And even the design file does not display this behaviour.
- We've decided to do away with breaks on descenders
- We will be triggering the focus state only when a keyboard accesses it.
@shadabbuchh - Issues 1, 3, and 4 should be fixed! I'm still not sure why 2 is happening, but since it's also happening on local-release and not anywhere else I think we can ignore it for now.
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3338492168.
Workflow: Appsmith External Integration Test Workflow.
Commit: 9a4160978cef148da079a90fff5c99f6a7e4d847.
PR: 17740.
Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-63465d4789020c7ac296d08d?pr=17740&runId=3338492168_1
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3342405203.
Workflow: Appsmith External Integration Test Workflow.
Commit: 9a4160978cef148da079a90fff5c99f6a7e4d847.
PR: 17740.
Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-63465d4789020c7ac296d08d?pr=17740&runId=3342405203_1
Additional bugs that have creeped in as a result of the previous fix:
- The gap between
Forgot Password,Back to Sign In,Privacy Policy&Terms & Conditionslinks & the underline beneath each of these is more than what is expected. - Letter spacing for
Privacy Policy&Terms & Conditionsis greater in the DP than release/prod. - Space between
Sign In&Forgot Passwordis greater than what is in release/prod. - The length of the whole
Sign In&Forgot Passwordcard is greater when compared to release/prod. - The placement of the
Sign Upcard as a whole is not congruent with release/prod. - Placeholder for Email/password is not placed in the exact center.
Please have a look at the video below for better reference: https://www.loom.com/share/8faa6e8dc47b4ed6a817a43da421f789
Placeholder for Email/password is not placed in the exact center.
This is not within the scope of the current PR (I haven't changed anything about that here)
The gap between Forgot Password, Back to Sign In, Privacy Policy & Terms & Conditions links & the underline beneath each of these is more than what is expected.
The space between the text and the underline is not determined by me, but determined by the line-height of the element which forms the bounding box of it. This will therefore change for different elements (this is one of the downsides of using something other that text-decoration to style our underlines). Since the difference is proportional to the font size, I propose that this is acceptable.
Letter spacing for Privacy Policy & Terms & Conditions is greater in the DP than release/prod. Space between Sign In & Forgot Password is greater than what is in release/prod.
This is an excellent catch, thank you! I have now implemented TextType within Link so this should be consistent everywhere.
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3382623903.
Workflow: Appsmith External Integration Test Workflow.
Commit: c43b9c0fdef97a88d9a8e961bc31823b9eb07ee4.
PR: 17740.
Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-63465d4789020c7ac296d08d?pr=17740&runId=3382623903_1
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3476883272.
Workflow: Appsmith External Integration Test Workflow.
Commit: d3cb862f88475651a006e9a8aab14b2b3f22c1a8.
PR: 17740.
Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-63465d4789020c7ac296d08d?pr=17740&runId=3476883272_1
Tests running at: https://github.com/appsmithorg/appsmith/actions/runs/3488282158.
Workflow: Appsmith External Integration Test Workflow.
Commit: 9cb04a2755da70ad660dd33fdba82c56657fb64f.
PR: 17740.
Perf tests will be available at https://app.appsmith.com/app/performance-infra-dashboard/pr-details-63465d4789020c7ac296d08d?pr=17740&runId=3488282158_1