Skip to content
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

optimize for mobile and clean up design #79

Merged
merged 39 commits into from
Jun 6, 2019
Merged

optimize for mobile and clean up design #79

merged 39 commits into from
Jun 6, 2019

Conversation

gesquinca
Copy link
Contributor

No description provided.

@MikeLockz
Copy link
Contributor

image

On desktop: 4 different left alignments - should these all be in the same general container / left alignment?

image

Does this need padding around the text?

image

Do we want to show QR next to blockie? This seems confusing.

image

How can we update this table layout to not side scroll? Can we trim out some unnecessary info? The current date doesn't make much sense here since there isn't historical data - this table is only for your current session. Do we need my local time zone? Time zone offset? Do we need the full hash or can we use the shortened?

I like the status pill. Can we update the color based on status? If it fails can we make red? On success green?

image

On mobile in MetaMask app - can't click the connect button. This was fixed in the previous layout.

The CTA button should always be visible without scrolling. Previously the button was fixed position to the bottom of the modal with the content area scrollable. See the implementation on the LowFunds modal for an example.

Couldn't get past the Verify modal, I think something might have changed with MM that broke this. I thought it was working before.

image

On mobile cipher browser - same issue with Connect button in modal. Seems like the modal content area is scrollable here and the button is fixed to the bottom but the positioning or something is off.

image

Do ToastMessage icons not show up on mobile?

image

Not obvious that this table is scrollable on mobile. Should we change the orientation of the data from left-right to top-bottom on mobile?

@as-dr
Copy link

as-dr commented May 30, 2019

image

On desktop: 4 different left alignments - should these all be in the same general container / left alignment?

I don't know if all of them need to be aligned with each other, but simplifying alignment of multiple elements would be cleaner.

image

Does this need padding around the text?

Consistent padding would be good, e.g. padding: 24px; or padding: 1.5rem;

image

Do we want to show QR next to blockie? This seems confusing.

Agreed. Resizing one of these to establish a hierarchy would be good.

FWIW, Dharma integrate the QR code & address like this:

Screen Shot 2019-05-30 at 4 53 41 PM

I don't know if you want to do that, but it's an alternative that could establish the blockie as the primary element in the hierarchy.

image

How can we update this table layout to not side scroll? Can we trim out some unnecessary info? The current date doesn't make much sense here since there isn't historical data - this table is only for your current session. Do we need my local time zone? Time zone offset? Do we need the full hash or can we use the shortened?

Anecdotally, I've seen a lot of apps use "table scrolling" at smaller viewports. We might consider this "best practices".

I like the status pill. Can we update the color based on status? If it fails can we make red? On success green?

👍

@gesquinca gesquinca closed this Jun 4, 2019
@gesquinca gesquinca reopened this Jun 4, 2019
@gesquinca
Copy link
Contributor Author

image

On desktop: 4 different left alignments - should these all be in the same general container / left alignment?

I don't think they need to be aligned. It doesn't bother me visually in it's current state, and while it certainly could be better, I think this is an improvement over the previous state and we should move forward.

image

Does this need padding around the text?

Padding would be nice. However, I would like to move this div outside of the tbody element because that's an invalid html structure. I would also like to make this an alert, because that seems to make more sense in this context. Again, I think this a question of incremental improvements and time.

image

Do we want to show QR next to blockie? This seems confusing.

I don't think any of these design changes are necessarily ideal, but I do think they are incremental improvements. I'd be curious to know what would be the point of confusion here? It doesn't seem confusing to me.

image

How can we update this table layout to not side scroll? Can we trim out some unnecessary info? The current date doesn't make much sense here since there isn't historical data - this table is only for your current session. Do we need my local time zone? Time zone offset? Do we need the full hash or can we use the shortened?

What is the issue with side scrolling? I made these changes to give the user more valuable data is readable and accessible. I think this is an improvement over what we had previously.

I like the status pill. Can we update the color based on status? If it fails can we make red? On success green?

Yes, we could. That would be nice. Is it crucial? Is this better or worse than before as is? Do we hold this deploy for it, or add it to list of nice-to-haves?

image

On mobile in MetaMask app - can't click the connect button. This was fixed in the previous layout.

The CTA button should always be visible without scrolling. Previously the button was fixed position to the bottom of the modal with the content area scrollable. See the implementation on the LowFunds modal for an example.

We should fix for sure. I'm don't think that fixed positioning is the best solution, but I'm sure there is one.

Couldn't get past the Verify modal, I think something might have changed with MM that broke this. I thought it was working before.

We should look into this issue and find the root of the problem.

image

On mobile cipher browser - same issue with Connect button in modal. Seems like the modal content area is scrollable here and the button is fixed to the bottom but the positioning or something is off.

Layout issue. Will probably be solved with fixing the CTA button issue above.

image

Do ToastMessage icons not show up on mobile?

They do not. This has always been the case.

image

Not obvious that this table is scrollable on mobile. Should we change the orientation of the data from left-right to top-bottom on mobile?

This seems like another nice-to-have that shouldn't hold up the current incremental improvements. Again, not my ideal solution, but my idea of an improvement over the previous state.

Changing the orientation of the date does not seem like a trivial change to make for me, but I could be missing something here.

@gesquinca
Copy link
Contributor Author

  • constrain all the ui to the same width
  • add padding to text inside table
  • remove blockie
  • add bottom padding to compensate for MetaMask browser ui bug

@gesquinca
Copy link
Contributor Author

resolves #73 #77

@gesquinca gesquinca requested review from MikeLockz and ryancreatescopy and removed request for MikeLockz June 5, 2019 14:00
Copy link
Contributor

@MikeLockz MikeLockz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

@MikeLockz MikeLockz merged commit b1c5b03 into master Jun 6, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants