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

WIP: Small UI Improvements Phase 2 #629

Closed
wants to merge 6 commits into from
Closed

Conversation

SaraVieira
Copy link
Collaborator

@SaraVieira SaraVieira commented Oct 6, 2017

The latest demo: https://styleguide-vwetycents.now.sh/

What is done:

  • Carret added on Props to help people understand that it's a clickable drop-down
  • Sidebar now closes and opens with the click of a button 🎉
  • Toggle props button is in place
  • Responsive Navigation, when the user is on a phone the navigation will hide and in his place the user will only see the show sidebar button to toggle it.

What I need help:

  • Understand the best way to make toggle props master button work
  • Fix commented test
  • Feedback

Closes #426 & #424

@SaraVieira SaraVieira requested a review from sapegin October 6, 2017 22:02
@SaraVieira
Copy link
Collaborator Author

I know the build is failing , any idea what values used to be ?

@sapegin
Copy link
Member

sapegin commented Oct 7, 2017

I know the build is failing

It says the build is now two times bigger than it used to be.

Copy link
Member

@sapegin sapegin left a comment

Choose a reason for hiding this comment

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

I like how it works!

Carret added on Props to help people understand that it's a clickable drop-down

They are tabs, not dropdowns. How would it look with more than one tab? In any case the Code tab should look the same way and the arrow icons should be inside the tab button component.

Toggle props button is in place

Now it’s hidden in isolated mode — where we need it most ;-)

Understand the best way to make toggle props master button work

I think it should be more like the real isolated mode. What would you like to see when you develop? Just props aren’t a big issue now because they are hidden by default. So probably we should hide text and description, make the heading smaller, etc.

And please remove yarn lock file.

@SaraVieira
Copy link
Collaborator Author

SaraVieira commented Oct 7, 2017

  • - Code tab should look the same way and the arrow icons should be inside the tab button component.
  • - remove yarn lock file.
  • - Toggle props is hidden in isolated mode
  • - Sidebar button should not be in isolation mode

On it !

It says the build is now two times bigger than it used to be.

I know but why ? I didn't even install anything

I think it should be more like the real isolated mode. What would you like to see when you develop? Just props aren’t a big issue now because they are hidden by default. So probably we should hide text and description, make the heading smaller, etc.

You think the click should make changes to the theme ?

@sapegin New demo: https://styleguide-xtiemmtyfv.now.sh

@ankri
Copy link
Collaborator

ankri commented Oct 7, 2017

I really like the new looks! Could you please also add a title attribute to all the toggles and buttons? I don't think there is a need for a separate tooltip dependency but titles for example would have helped me to understand what this button is for:

image

@sapegin
Copy link
Member

sapegin commented Oct 7, 2017

Yeah, we just use title for all other toolbar buttons.

@sapegin
Copy link
Member

sapegin commented Oct 7, 2017

I’m not giving much feedback about the code because we’re not yet sure how everything should look. But I’ll read it carefully later ;-)

I know but why ? I didn't even install anything

That’s something weird. I only see two new icons, something must be wrong here. I’ll take a look later.

You think the click should make changes to the theme ?

I don’t think so. Maybe we should hide the header and show component name somewhere else, like on a fixed toolbar near buttons to toggle sidebar and toggle uberisolated mode. I’m open for ideas ;-)

/cc @okonet @kof @tizmagik @n1313 @stepancar

@SaraVieira
Copy link
Collaborator Author

@sapegin I could do a tooltip in just CSS and the content in the before, what you think?

@SaraVieira
Copy link
Collaborator Author

SaraVieira commented Oct 7, 2017

I'll do some iterations on the focus button of that and send some screenshots :)

@sapegin
Copy link
Member

sapegin commented Oct 7, 2017

Maybe a good idea but as a separate PR ;-)

@sapegin
Copy link
Member

sapegin commented Oct 7, 2017

Also use ToolbarButton bur buttons — right now it’s just an image.

@SaraVieira
Copy link
Collaborator Author

What do you mean by the last comment?

@sapegin
Copy link
Member

sapegin commented Oct 7, 2017

You’re attaching an onClick handler to an image which make them inaccessible, inconsistent with other toolbar buttons, you have to write a lot of extra styles, and we already have a component for toolbar buttons.

@SaraVieira
Copy link
Collaborator Author

SaraVieira commented Oct 7, 2017 via email

@SaraVieira
Copy link
Collaborator Author

@ankri @sapegin Added the toolbarButton and it looks waaaay better actually :p

thank you for reminding me 👍

https://styleguide-vwetycents.now.sh/

Will take a lot at some possible mods tomorrow for focus mode :)

@okonet
Copy link
Member

okonet commented Oct 11, 2017

@SaraVieira here is my thoughts on the UI:

  1. I think the location of the buttons in the sidebar is odd. It was merely an accident I noticed them.
  2. I'm wondering why do we even need the hide sidebar button?
  3. The second button doesn't do anything for me...

@SaraVieira
Copy link
Collaborator Author

@okonet Thanks for the feedback! 🙌

Regarding the second button it's true it doesn't do anything yet :(

Where do you think it would be a better position for them?

About the hiding the sidebar it has come in handy for me a couple of times but yes it's a niche problem

@sapegin
Copy link
Member

sapegin commented Oct 11, 2017

@SaraVieira Any particular use cases for manually hiding the sidebar? I really want the uberisolated mode ;-)

@SaraVieira
Copy link
Collaborator Author

The main use case is mobile like it does right now, besides that it could be useful if you want to work in focus mode.

Now that I think about its more of a pretty thing then an actually necessity on desktop 🙁

@sapegin
Copy link
Member

sapegin commented Oct 11, 2017

But do you really need to see all components in that mode?

@SaraVieira
Copy link
Collaborator Author

Mostly not 🙁

Maybe the sidebar removal in desktop should enable focus mode instead of having two buttons

@SaraVieira
Copy link
Collaborator Author

Btw, @sapegin I pinged you on slack

@sapegin
Copy link
Member

sapegin commented Oct 11, 2017

I think the main question here is what would be the best experience for developing components?

For me it would be uberisolation mode:

  • one component
  • all examples visible at the same time
  • No distractions like a huge heading, pathline, etc.
  • Togglable side paddings in example playgrounds (for responsive testing)

@okonet
Copy link
Member

okonet commented Oct 28, 2017

Yes, let's approach the problem with real use cases and not with the UI first. I'll write down my thoughts on the DX as soon as I get some more time. Sorry for delays :-/

@tizmagik
Copy link
Collaborator

Yes, for me the most important DX consideration is lack of chrome (padding, etc) for responsive testing.

@SaraVieira
Copy link
Collaborator Author

@tizmagik I can tackle that and just get a better mobile experience for testing

@okonet Not a problem man! Get back to this when you have time :)

@sapegin
Copy link
Member

sapegin commented Nov 17, 2017

I’d suggest to split this PR otherwise we’ll never finish it because there are so many things that needs further discussion.

Carret added on Props to help people understand that it's a clickable drop-down

+1 from me.

Sidebar now closes and opens with the click of a button 🎉

Not sure we really need that.

Toggle props button is in place

That’s the whole discussion of the uberisolated mode.

Responsive Navigation, when the user is on a phone the navigation will hide and in his place the user will only see the show sidebar button to toggle it.

This is useful I think but since we’re not sure about the toggle sidebar button I’m not sure how it should work.

What do y’ll think?

My thoughts and wishes on uberisolation:

image 2017-11-17 at 1 14 49 pm

I also think it should be a permanent toggle between isolation and uberisolation (and same for full-width mode), that would be stored in localStorage, so every time you will open isolation of a component you’ll have your last setting.

Any feedback?

@stale
Copy link

stale bot commented Dec 28, 2018

😴 This issue has been automatically marked as stale because it has not had recent activity. It will be closed in a week without any further activity. Thank you for your contributions.

@stale stale bot added the wontfix label Dec 28, 2018
@stale stale bot closed this Jan 4, 2019
@sapegin sapegin deleted the ui-minor-improvements branch November 11, 2020 16:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UI improvements proposal (UI 2.0)
5 participants