-
Notifications
You must be signed in to change notification settings - Fork 176
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
chore: Update staking farm related copy #2388
Conversation
MaximusHaximus
commented
Jan 20, 2022
- Alphabetizes the JSON files now that we have automatic tooling added to handle this whenever they are edited
- Updates copy for token farm validators
Your Render PR Server URL is https://near-wallet-pr-2388.onrender.com. Follow its progress at https://dashboard.render.com/static/srv-c7kb22s6fj3bh06aqb8g. |
@corwinharrell Would like your input on the new tooltip text and the new balance of tokens earned via the staking farm.
|
@MaximusHaximus Could you add screenshots of the affected UI in context? Just helps to review things like this without going through setup locally. Would love to get into a habit of this any time there are changes that affect the front-end/UI |
@corwinharrell You don't need to set anything up locally to see how changes look in context, since we automatically deploy previews for every PR with both testnet and mainnet configurations.
Here's a picture of the screen with the new terms being rendered for your convenience, but it would probably be a good idea for you to get in the habit of using the preview builds in general, since screenshots are pretty limited (especially for changes that can impact multiple screens) |
Sorry for the miscommunication @MaximusHaximus. I'm totally comfortable using the deploy links. My comment about adding an image was purely an aside and for convenience sake for future PRs. Didn't mean to suggest it was a blocker. Had to step away for a bit this afternoon but looking at this now. |
@MaximusHaximus See this comment: #2383 (comment) since I think it has a direct impact on the changes proposed in this PR. |
Also added loading 'dots' so that it is clear when we are still fetching farm details
…r is provided as a prop (equal spacing between entries)
43cfb76
to
eb61c50
Compare
@corwinharrell I modified the UI to look like the picture you've created, and modified the translation terms accordingly -- thanks for the feedback/direction! |
@Patrick1904 I'm pretty sure my usage of dynamic props for the container here isn't what you'd want given I haven't seen this pattern in other code -- I'm shipping with that setup in the interest of time but am keen to have a chat with you when you're back about the right way to handle the border formatting in the list 'the right way, and to understand the benefits of avoiding dynamic props usage for our style components if that's what we're generally trying to do as a team. :) |