Open sschiessl-bcp opened 5 years ago
Thanks for taking this @calvinfroedge
Let us know what you find out!
@sschiessl-bcp Working on it now
@sschiessl-bcp I do like this solution proposed in the link you added:
Currently exploring!
@sschiessl-bcp @startailcoon @gibbsfromncis
I have a proposed solution ready:
import {Tooltip, Icon} from "antd";
import React from "react";
class BitsharesTooltip extends React.Component {
state = {
isMobile: false
}
componentDidMount(){
if(window.innerWidth < this.props.mobileBreakpoint){
this.setState({isMobile: true});
}
window.onresize = ()=>{
if(window.innerWidth < this.props.mobileBreakpoint && !this.state.isMobile){
this.setState({isMobile: true});
} else if(window.innerWidth >= this.props.mobileBreakpoint && this.state.isMobile){
this.setState({isMobile: false});
}
}
}
render(){
let { props, state } = this;
let { withQuestionIcon } = props;
let { isMobile } = state;
let extraProps = {};
if(isMobile){
extraProps.trigger = "click";
extraProps.placement = "topLeft";
}
return withQuestionIcon ? <div style={{position: "relative"}}>
<div style={{position: "absolute", zIndex: "1", right: "5px", top: "5px"}}>
<Tooltip {...props} {...extraProps}>
<Icon type="question-circle" theme="outlined" style={{fontSize: "1.5em"}}/>
</Tooltip>
</div>
{this.props.children}
</div> : <Tooltip {...props} {...extraProps} />
}
}
BitsharesTooltip.defaultProps = {
mobileBreakpoint: 760
}
export default BitsharesTooltip;
Basically a breakpoint gets passed to the tooltip (default of 760) which determines whether it opens on click (mobile, below 760) or on hover (the default). Also as there are the ? icons for the inputs it provides a prop which will add the question icon and position it over an element. May be a good idea to provide some additional props for overriding that (or just the style prop). But this is proof of concept, looking for feedback.
Looks great, but do we need the tooltip without the question mark symbol? I only see the usage of the first type. If extra info is available for an input, the icon is present, and it signals to the user that there is one.
If only the first type is used then that makes it easier =) I'll just make the positioning stuff adjustable via props. I thought maybe tooltips were available on non inputs as well. Thanks @startailcoon
@gibbsfromncis @startailcoon @wmbutler Can someone get me access to style guide repo so I can push my branch or should I fork it and PR?
@sschiessl-bcp can you grant access to @calvinfroedge ? Otherwise @calvinfroedge it's perfectly legit to fork and PR.
@startailcoon Took your advice, now just question mark, also allowed most of the special props to overwrite:
import {Tooltip, Icon} from "antd";
import React from "react";
class BitsharesTooltip extends React.Component {
state = {
isMobile: false
}
componentDidMount(){
if(window.innerWidth < this.props.mobileBreakpoint){
this.setState({isMobile: true});
}
window.onresize = ()=>{
if(window.innerWidth < this.props.mobileBreakpoint && !this.state.isMobile){
this.setState({isMobile: true});
} else if(window.innerWidth >= this.props.mobileBreakpoint && this.state.isMobile){
this.setState({isMobile: false});
}
}
}
render(){
let { props, state } = this;
var { wrapperStyles, iconStyles } = props;
let { isMobile } = state;
let extraProps = {};
if(isMobile){
extraProps.trigger = "click";
extraProps.placement = "topLeft";
}
wrapperStyles = Object.assign({
position: "absolute",
zIndex: "1",
right: "5px",
top: "5px"
}, wrapperStyles || {});
iconStyles = Object.assign({
fontSize: "1.5em"
}, iconStyles || {});
return <div style={{position: "relative"}}>
<div style={wrapperStyles}>
<Tooltip {...props} {...extraProps}>
<Icon type="question-circle" theme="outlined" style={iconStyles}/>
</Tooltip>
</div>
{this.props.children}
</div>
}
}
BitsharesTooltip.defaultProps = {
mobileBreakpoint: 760
}
export default BitsharesTooltip;
First solution aimed to be more flexible and assume that there may be use cases other than inputs in the future, which @sschiessl-bcp alluded to in the issue description. I'm fine with either.
@calvinfroedge You are part of ui-dev team, which has write access to style guide. What was the error?
@sschiessl-bcp Something is different...
bitshares-ui
git push -u origin 2230-mobile-tooltips
success
bitshares-ui-style-guide
git push -u origin 2230-mobile-tooltips
403
Please try again.
master is now also a protected branch
PR is open on style guide... https://github.com/bitshares/bitshares-ui-style-guide/pull/5
What's the quickest why I can play around locally with that?
1) Download style guide repo https://github.com/bitshares/bitshares-ui-style-guid
2) Checkout branch 2230-mobile-tooltips
3) Click on one of the themes and scroll down to tooltips demo
Zhe behavior if onHover or onClick depends on how the website was when it is first opened, and remains like that when resized. Is that on purpose?
Additionally, it looks misplaced on my end
The issue with tooltip width is I think an Ant issue. You'll see (I think) the same behavior on unmodified ant tooltips. The Ant tooltips don't behave completely properly during resize.
But yes, onHover vs onClick is supposed to depend on the page width, and there is a param to give a mobile breakpoint.
@sschiessl-bcp Bump
a) What is best practice when it comes to breakpoints and rendering? Right now, the page uses the click/hover behavior depending on how big the site was when it was opened, not the actual current size. I feel it should behave according to current size, just that we are aware of our choice
b) A mobile tooltip without a question mark makes no sense as the user would not see it, and once the user knows what to do, popping up the tooltip everything is breaking UX IMO
c) The original issue asked for three styling options, can you please include examples for each of those in the example? For example below User must be able to see that tooltip on mobile as well. This would be a case of a randomDOM where the user must say where the "?" icon is displayed in case of mobile rendering. It could be hovering over the right side of the button (just an example, we need some kind of mobile way though) and the tooltip appears when the user presses on the right side without releasing the button, or simply onclick in the right part of the button
d) would an additional props forceMobile
be usefull? Then we could show one set of tooltips in the style-guide and below the same set with force mobile on
@sschiessl-bcp In the issue description, which I confirmed with @startailcoon, the issue was only to develop a new standard tooltip, not to implement it everywhere.
a) What is best practice when it comes to breakpoints and rendering?
Right now, the page uses the click/hover behavior depending on how big the site was when it was opened, not the actual current size. I feel it should behave according to current size, just that we are aware of our choice
It does respond to the current size, but a bug in ANT prevents the tooltips from being sized correctly when the window is resized a lot.
b) A mobile tooltip without a question mark makes no sense as the user would not see it, and once the user knows what to do, popping up the tooltip everything is breaking UX IMO
This is the same thing @startailcoon said and I agreed with him. @gibbsfromncis had a more purist idea about doing exactly what ant does by default and said I broke all of his tooltips. For what it's worth, I don't agree that a tooltip that opens when a field is selected is worthless.
c) The original issue asked for three styling options, can you please include examples for each of those in the example?
User must be able to see that tooltip on mobile as well. This would be a case of a randomDOM where the user must say where the "?" icon is displayed in case of mobile rendering. It could be hovering over the right side of the button (just an example, we need some kind of mobile way though) and the tooltip appears when the user presses on the right side without releasing the button, or simply onclick in the right part of the button
You can override the positioning prop as with normal ant component by sending placement
prop. Where the question mark appears is more of a styling concern for each individual instance.
d) would an additional props forceMobile be usefull? Then we could show one set of tooltips in the style-guide and below the same set with force mobile on
Right now there is the withQuestionIcon
prop. In a previous version there was only the question icon (as you and @startailcoon both raised the same concern). If it's mobile, literally the only forced difference is a default placement of topLeft
which can be ridden and open on click which was written into the issue requirements and can't be overwritten.
If it's mobile, literally the only forced difference is a default placement of topLeft
Also the onClick / onHover behavior is different, and stays like that even if resized
the issue was only to develop a new standard tooltip, not to implement it everywhere.
Certainly! But can I cover all three cases I have described in the issue?
For what it's worth, I don't agree that a tooltip that opens when a field is selected is worthless.
Agreed, might be cases that its usable
Where the question mark appears is more of a styling concern for each individual instance.
There are cases like described above where I want no question mark on desktop view, but I want one on mobile view. So in that case I need to tell the Tooltip, if you are mobile, put the circle here
@sschiessl-bcp @calvinfroedge what is the progress on this?
@sschiessl-bcp @startailcoon I stopped working on this as I had other things that came up with and getting the issue closed was taking a while. I can look back into closing out this issue if it's still a need.
@calvinfroedge sure!
Since we will use the same components on any new designs, I'd say we could try to get this merged. Correct @gibbsfromncis?
@startailcoon sure. I'll take a look on PR in style-guide
repo and check it. If something is wrong I'll fix and merge it.
Progress on this, can we merge it with our current repo? @gibbsfromncis
@startailcoon I didn't have time to review it. @react-cat do you mind to take it?
@gibbsfromncis Sure, I'll take a look at it soon
@startailcoon, @gibbsfromncis There is a bug, in case you want to use popup without question icon, tooltip get's a body as a DOM node to be bind to. I tried a lot of things and make a research in ants community and there is no easy way to create correctly working input.
We can handle a click/hover/focus on children and toggle visibility of tooltip manually, but it would require a lot of work, test etc. so I think it's better to end up with one input with question sign.
Also after review I noticed some useless code and some places that need refactor or improvements.
Let me know if we really need the second input and if you have any thoughts about it.
@react-cat I'll take care of it tomorrow
I'm lost what the current approach here is, and if it is what we need. Can someone please recap and point me where I can test it?
Playing around with the latest spotlights it is quite hard to define a tooltip that is suitable for mobile. Current thought is to have a "?" icon that can be clicked, and if clicked in inserts a span or label above the component with the text. I find mobile tooltip as popup more and more unsuitable.
We need a solution for mobile view (or any touch screen), I quite like this one http://uxmovement.com/mobile/how-to-display-tooltips-on-mobile-forms/ Tooltips are rendered as separate button, on click shows the text.
Tooltips are integrated as a wrapper document.
Different styling come to mind:
children
is an icon. Display the icon with an easy to click size and give it onClickchildren
is a label for a textfield. The tooltip should be rendered as "?" button within the textfieldchildren
is any random DOM with several subcomponents. Tooltip icon could be positions top|left|right|bottom aligned, or floating over.This issue is to investigate best practices and work out an abstract and flexible solution.