Revert "Zero all padding for BT, VGS, and Rosetta elements" #276
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. Join @devinmorgan and the rest of your teammates on |
devinmorgan
commented
Jun 18, 2024
There was a problem hiding this comment.
I made this a PR so to make it easier for folks to find this branch; it's just supposed to demo what the different vaul elements looked like before the styling changes. However, the best way to demo it required modifying the ForagePINEditText in a way that we don't actually want. That's why I'll be closing this once if #247 is accepted.
This reverts commit ed486f0.
devinmorgan
force-pushed
the
devin/demo-with-styling-partity
branch
from
3323559 to
bd7e89d
Compare
devinmorgan
force-pushed
the
devin/demo-with-NO-styling-parity
branch
from
bf6bee8 to
e71562e
Compare
devinmorgan
added a commit
that referenced
this pull request
Jun 18, 2024
<!-- Update your title to prefix with your ticket number --> ## What Ensure the base appearances of the VGS, BT, and Rosetta text elements are identical. The main problem was that VGS and BT were setting default patterns. This PR zeroes out the paddings of all of our vaults. It's worth mentioning that padding has the identical visual effect as adjusting the width and height of the text element when the text is horizontally and vertically centered. For this reason, I believe zeroing out the padding is an acceptable solution. <!-- Please include a summary of the change. List any dependencies that are required for this change. --> ## Why Forage needs to route traffic to different vault providers transparently based on availability. We were facing a problem where swapping out the vault providers was _not_ transparent when `inputHeight` or `inputWidth` attributes were not supplied to `ForagePINEditText`, a valid SDK usage. <!-- Describe the motivations behind this change if they are a subset of your ticket --> ## Test Plan This was a purely appearance change, so I did not add any tests. ## Demo NOTE: The demo images are from two other branches that modified `ForagePINEditText` to render VGS, BT, and Rosetta side-by-side. We don't want those demo-changes in `main` so that demo-code lives in separate branches that won't get merged in (i.e. #275 and #276) ### Before - No Styling Parity > 📌 To view this demo for yourself, pull `devin/demo-with-NO-styling-parity`  ### After - With Styling Parity > 📌 To view this demo for yourself, pull `devin/demo-with-styling-partity`  <!-- If applicable, please include a screenshot to this PR description --> <!-- If applicable, please include a screen recording and post it in #feature-recordings --> ## How Let's discuss what we think about this change. Once we're all aligned, this PR can be merged in. NOTE: I will be closing out #275 and #276. They were only meant to serve as demos and not actually meant to be merged. <!-- Describe the rollout plan if it includes multiple PRs/Repos or requires extra steps beyond reverting this PR -->
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
Why
Test Plan
Demo
How