Skip to content

Conversation

@eps1lon
Copy link
Member

@eps1lon eps1lon commented Feb 23, 2019

Puts emphasis on the toggle demo source button when hovering over the demo itself.

I experimented with opacity but this doesn't translate very well to the dark theme: Light theme has default .54 opacity meaning we can highlight it by increasing it. However, the dark theme already is at 1 so we can't emphasize it more.

I settled on switching from the default color to primary when hovering over the demo. This is following material design.

This also includes showing the tooltip on hover. This features is disabled for now. Should we only display the tooltip when the toggle was never clicked? How do we persist this? cookies, localStore?

@eps1lon eps1lon added the docs Improvements or additions to the documentation. label Feb 23, 2019
@eps1lon eps1lon requested a review from mbrookes February 23, 2019 10:21
@mui-pr-bot
Copy link

mui-pr-bot commented Feb 23, 2019

Details of bundle changes.

Comparing: bfa9167...f93c992

bundle parsed diff gzip diff prev parsed current parsed prev gzip current gzip
@material-ui/core 0.00% 0.00% 369,384 369,384 91,457 91,457
@material-ui/core/Paper 0.00% 0.00% 76,647 76,647 19,297 19,297
@material-ui/core/Paper.esm 0.00% 0.00% 71,595 71,595 18,771 18,771
@material-ui/core/Popper 0.00% 0.00% 30,462 30,462 10,584 10,584
@material-ui/core/styles/createMuiTheme 0.00% 0.00% 17,286 17,286 5,720 5,720
@material-ui/core/useMediaQuery 0.00% 0.00% 2,486 2,486 1,054 1,054
@material-ui/lab 0.00% 0.00% 182,793 182,793 50,217 50,217
@material-ui/styles 0.00% 0.00% 57,708 57,708 16,237 16,237
@material-ui/system 0.00% 0.00% 17,062 17,062 4,486 4,486
Button 0.00% 0.00% 99,068 99,068 26,484 26,484
Modal 0.00% 0.00% 98,649 98,649 26,152 26,152
colorManipulator 0.00% 0.00% 3,232 3,232 1,297 1,297
docs.landing 0.00% 0.00% 49,899 49,899 10,728 10,728
docs.main +0.07% 🔺 +0.08% 🔺 677,075 677,542 205,583 205,746
packages/material-ui/build/umd/material-ui.production.min.js 0.00% 0.00% 320,511 320,511 84,731 84,731

Generated by 🚫 dangerJS against f93c992

@oliviertassinari
Copy link
Member

oliviertassinari commented Feb 23, 2019

I like the idea, my favorite solution is probably https://semantic-ui.com. They show the source code tooltip when one demo is hovered. As soon as the user interacts with the icon button, they increase his expertise level with a cookie:

capture d ecran 2019-02-23 a 13 14 06

The tooltip will never be shown again.

@eps1lon
Copy link
Member Author

eps1lon commented Feb 23, 2019

The tooltip will never be shown again.

That is the part that I've currently disabled by default (showSourceHint). I can go with a cookie. I would name it different though. Something like knowsAboutDemoSource. expertiseLevel is to generic IMO and implies an order.

@eps1lon
Copy link
Member Author

eps1lon commented Feb 23, 2019

@oliviertassinari 354f9df

@eps1lon eps1lon force-pushed the docs/show-source-hint branch from 1be9fd4 to b10b6f9 Compare February 23, 2019 19:00
@mbrookes
Copy link
Member

mbrookes commented Feb 23, 2019

Something like knowsAboutDemoSource

sourceHintSeen ?

When I view the source for demo, it no longer shows the tooltip for that demo, but it does for all others on the page. Is that intended / unavoidable?

@eps1lon
Copy link
Member Author

eps1lon commented Feb 23, 2019

sourceHintSeen ?

That would imply it should only be shown on the first hover. Is this intended? Might be confusing if people hover over the demo very quickly and the tooltip flashes. Now it won't be shown again. I feel like coupling it directly to "the user actually used this once" is the safer route here. Although it might be annoying if people know about it and never use it.

@mbrookes
Copy link
Member

I was just suggesting something more concise for the cookie name. The "and interacted with it" part is implicit - that's how we know it was seen, not just displayed.

@eps1lon
Copy link
Member Author

eps1lon commented Feb 23, 2019

I guess so. knowsAboutShowSource does sound akward.

@mbrookes
Copy link
Member

In case it was missed because I edited my comment, rather than adding a new one:

When I view the source for demo, it no longer shows the tooltip for that demo, but it does for all others on the page. Is that expected / unavoidable?

@eps1lon
Copy link
Member Author

eps1lon commented Feb 23, 2019

In case it was missed because I edited my comment, rather than adding a new one:

When I view the source for demo, it no longer shows the tooltip for that demo, but it does for all others on the page. Is that expected / unavoidable?

Right. Since the other demos didn't update they still have the outdated cookie value.

Might be a better idea to put this into a store anyway and initialize from there. Basically set up an app wide state about capabilities the user has discovered. This touches on a related issue I have with handling the theme currently.

Since this issue only occurs on the first interaction with it I would keep this as-is and revisit it later when we improve state initialization from cookies.

@oliviertassinari
Copy link
Member

oliviertassinari commented Feb 23, 2019

@eps1lon We should be able to use the optionsReducer.js to store this new option and the SideEffectsRaw component to update it from cookies. The infrastructure is already in place (and can be improved too), it should be straightforward.

@eps1lon
Copy link
Member Author

eps1lon commented Feb 24, 2019

@eps1lon We should be able to use the optionsReducer.js to store this new option and the SideEffectsRaw component to update it from cookies. The infrastructure is already in place (and can be improved too), it should be straightforward.

This is what I'm getting at. The current infrastructure is problematic (throwaway component that has no clear purpose) and has some actual issues. I need to gather my thoughts about this later.

@eps1lon eps1lon merged commit 2215e40 into mui:next Feb 24, 2019
@eps1lon eps1lon deleted the docs/show-source-hint branch February 24, 2019 08:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Improvements or additions to the documentation.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants