-
Notifications
You must be signed in to change notification settings - Fork 2
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
feat: Moving form and result to two cols #47
base: main
Are you sure you want to change the base?
Conversation
|
@jhammond2012 @chadoh It seems that the preview isn't working for me in brave or firefox. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can't figure out why the PR Preview isn't showing up, but I was able to look at it locally. It's looking pretty good. My biggest current concerns are:
- most pages are now too wide, since the
--max-width
variable is used bycontainer
, which is used all over, such as on the home page - feels pretty jumpy. for example, flashes of wide content before content finishes loading, followed by a repaint to narrow layout. even in the best cases, this feels a little disorienting to me. This might be hard to fix in the general case; I'm up to have a call to chat through possible solutions.
@chadoh I moved the container class around a little bit so I could dynamically add |
This one still jumps at first page load: http://localhost:3000/#/v2.tenk.testnet/RemainingAllowance |
I added a list of things to check to the PR description, and checked off the ones that look good to me. The jumping is much more tolerable in this version, since it happens less, and only at more expectable times. Still noticing it here and there though, as mentioned above. You'll notice various issues as you click through. Some of the biggest ones currently worth noting:
|
why so many |
That's a good question. Did you use NPM before instead? I haven't installed/changed anything regarding that. Only ran |
@jhammond2012 ah, right, this repo is not using yarn. I tried to find the old commit where we switched to npm. At the time it fixed a deploy bug; I think it might have been related to PR Preview deploys when using Gatsby or something like that. We're not using that stack anymore and could probably switch back to Yarn, but let's do that in its own PR and remove the |
Still a WIP but moving the form and results to two columns. Need to clean the code a bit and take care of the case where the result is really short. This causes a large gap between the form and result.
Chad's checklist of things to get looking 💯