ethavatar
ethavatar copied to clipboard
gitcoinco
WIP: New design
A short summary of work that's been done:
- [x] Added majority of UI styles and assets
- [x] "No MetaMask" states finished in App component
- [x] "Existing Avatar" styles mostly finished
- [x] Merging / Refactoring the image upload components (Form & Image)
- [x] About / Help Panels
- [x] Avatar Lookup
- [x] Mobile Responsive media queries
Some remaining tasks:
- [ ] Fancy scroll bars
- [ ] Remaining copy
- [x] OG Tags
I'd like some feedback on what I've done so far, and the provision of any additional assets or copy that can be provided.
Some further notes:
- I'm not sure there are facilities for Avatar Lookup in here yet, am I to develop those, or just the UI?
- Travis isn't configured correctly.
@cryptoquick Thank you for raising this PR! Regarding builds and travis... I'll try to get a dockerfile definition and travis configuration in here sometime today.
do yall want review now, or should i wait until this isn't WIP anymore?
is this deployed somewhere on the web or should i try to spin it up locally?
regarding " Avatar Lookup", the current codebase should do this correctly.. also @tarekskr did the intiial implementation and if you ask nicely he might answer your questions :0
@owocki I can spin this up on a cheap VPS for the time being.
It's more or less ready for review. I just need to implement the last three things and it will be complete. I'd like to do some of these things in parallel, and there will need to be some input from you regarding production copy and dapps using this, for example.
As for Lookup, I don't see "lookup" anywhere in the code, and I'm pretty sure it's not a feature of the current site.
I set the project up on a VPS: http://138.197.12.222:3000/
You may have click through a Google phishing warning. Luck of the draw from the IP pool, I suppose.
@cryptoquick could we make the the accordion transition happen at a shorter duration ? It feels laggy even thought it actually isn't :P
Also @PixelantDesign I assumed this would be using Muli as against Futura 🤔
@cryptoquick this is the lookup method: https://github.com/gitcoinco/ethavatar/blob/master/contracts/EthAvatar.sol#L15-L17
just tested it out.. looks like its really coming along! yay!
a few QA things
- some js errors
- when i do an upload, it never seems to quite leave the loading screen (though if i refresh, the image is there!)
- we need to get some copy in the 'what is it' and 'use it in your dapp', and help sections. i can provide this if needed!
- responsive design for menu needs some work
Great job @cryptoquick !
One issue for me: the Ethavatar specs specify that a set avatar could be changed at anytime, but the 'Change' button isn't visible for me if I already have an avatar set. I believe the 'Change' button should always be visible.
Good feedback. I'll make these changes soon. 👍
As for copy, I can take a stab at it if you like. Is there any other documentation on EthAvatar, like a formal spec?
the spec is sort of a working standard right now; https://github.com/ethereum/EIPs/issues/928 is the dialogue / design specification
Sorry for the late reply, Cryptoquick that's a brilliant work! You have done a great job.
A Few thought:
- at the moment the website has no overflow-x: hidden; but I guess is needed to avoid horizontal scrolling. ( macos with chrome )
- The animation are a little slow I think. Good pratice would be to keep them under 0.3ms
- When I login in metamask it does not refresh the page but I need to refresh manually
- On my laptop screen everything seems giant: logo, menu , base text size, but I am sure this is part of the media queries work
- When the logo is uploaded we need a change button, you can refer to this screen:
I've added the functionality and styles for the Lookup form. You can find the new changes on the demo server:
http://138.197.12.222:3000/
@owocki
- I've fixed the errors you posted. One I had to fix by updating a dependency; react-loading.
- I think this should be addressed by my latest changes... It's also a little difficult to see sometimes when you set gas too low.
- I've tried to improve the copy a bit, but I need a bit more guidance on this front.
- Responsive design has had a lot more work put into it now.
@tarekskr Good catch. I've made the fix.
@mbeacom How's that Travis work coming? It'd be nice to spin down my VPS demo server.
@lucaguglielmi I've tried to address your issues with responsive design as well as adding the change button. I've also used your 0.3s recommendation, but I'm a little worried they might be too quick.
Save for some scroll bars (which I've been putting off...) and some better copy than the placeholder copy, we should be nearly ready to go, at least, for another round of review.
Also, I'm not 100% happy with how we're using the Blockies / identicon component in Lookup.
I think it's about time I mark this as delivered though. I'm totally down for more revisions, of course!
Hello! Thanks again for your work!
Animations: The thing to consider here is that there are several animated panels and the user will probably open / close more than one.
On average it takes a human 230 ms to visually perceive something. If an animation is repeated for many interactions (like our panels are), a slower, more-perceivable animation is going to feel boring for most users, especially the second time that is triggered. We do not want to make them feel like they're waiting for the animation. I personally think the speed works well now :)
"Change" button: In my original concept when the user click "Change" I would expect it to work immediatly as a "BROWSE IMAGE" button, and if anytime in the app you drag a new image on the avatar it gets replaced. Sorry if this wasn't clear.
Your current solution ( Change button opens a new view where you can drag the photo or browse on click) works pretty well and I think we should keep it! One problem that I see is that the user cannot change his mind when "change button" is pressed. I suggest that in this view we add a small cancel link to go back to the original avatar image.
Good idea! I'll add a cancel link underneath. There might be a couple states where I'll need to make sure the state can be reset or cancelled.
If I open demo server on mobile, it said "NO CONNECTION TO THE ETHEREUM NETWORK" (this is normal). If I then click on Need Help, it open "popup" with help, but I can't scroll down to see other text in help.
@filips123 Good point. I've been putting off styling the scroll bars. Maybe I can find a scroll bars component. :)
just tested out the app.. its coming along really nicely!
one piece of feedback is that i think the navbar items are still a little bit too big at some resolutions screengrab
will make a note to mention this progress on the ethavatar call next time we have one. thanks @cryptoquick -- hit me up when you feel like this is ready for a merge!
@cryptoquick Scrool bar is now displayed, but disabled, even on desktop.
Which Ethereum network should I use. If I use Ropsten or Kovan, it displayed "Loading EthAvatar...". If these networks are not supported, this should be displayed in website.
In Firefox 60, text "An Avatar for your ETH address" is displayed too right and it's cropped. In Chrome or Edge, text is displayed normal.
At some resolutions, text is displayed over other text. Also, navbar is cropped in Firefox, Chrome and Edge.
Is it normal to crop "Your ETH Address:" element when using "Avatar Lookup" or other items in navbar?
I think the text in the navbar is too big.
Also, add support for Ethereum Name Service (ENS).
Alright, some of those items fall out of scope of what I'd like to achieve with this PR. If you like, @filips123 can you add issues for ENS? #10 is already present for Ropsten / Kovan support.
The other more QA-facing items I can absolutely accomplish. I'm hoping to button this one up over the weekend, so I can get out of @owocki 's hair and maybe I could get a kudos at the local meetup he's speaking at on Monday... hint hint. :smile_cat:
@PixelantDesign sure! I can bring this one over the finish line. I had trouble earlier due to getting a new job, but now that I'm settled in, I can put this one back on the front burner. 👍
BTW, I just realized I needed to host this on an HTTPS domain, otherwise there would be CORS errors connecting to Infura, so I threw something up real quick.
https://ethavatar.huntertrujillo.com/
I think the last few things I want to do at this point are:
- Copy... I'm not sure about images though, I'm not a graphic designer, and I wasn't sure if suitable images and copy are available in the composition. So, I guess I'm asking for help with this.
- Are fancy scroll bars a strict requirement for delivery?
- Also, we need a favicon, and an image to point the OG Tags to, like, a screenshot or something.
I guess that's what I left as tickboxes up top... Aside from that, I don't think there's anything else really that major that needs to be done. Am I wrong?
Hello! Looking great but can I suggest to make all font-sizes a little smaller? The text Is really big at the moment. Normal p text of 16px at all resolution is a good starting point I think.
Scroll bar with proper padding will be a great improvement i think, I can find one for you if you want
@lucaguglielmi The text will be larger the larger the screen you view it on. I've sized it to a system of vh units, for the most part. I don't recall having a responsive design, so that's what I did to make it a bit more responsive at different resolutions.
I haven't had consistent, real-world luck with scroll bars, so if you'd like to recommend some that have worked for you, please go ahead.
@cryptoquick Some of things that I notices in https://github.com/gitcoinco/ethavatar/pull/12#issuecomment-394152002 still not work.
Also, we need a favicon, and an image to point the OG Tags to, like, a screenshot or something.
You could probably use icon from https://ethavatar.huntertrujillo.com/images/logo.png for logo.
The text will be larger the larger the screen you view it on. I've sized it to a system of vh units, for the most part. I don't recall having a responsive design, so that's what I did to make it a bit more responsive at different resolutions.
I agree with responsive design, but I would also like smaller text. I don't test this on mobile, but on desktop, text is probably too big. Maybe add different text sizes using CSS media query max width?
@cryptoquick I've added support for EIP-1102 (fix gitcoinco/ethavatar#20) as PR to your new design repository (see cryptoquick/ethavatar#1). Could you please merge my PR and add it to this?