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.

Sign up for GitHub

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

[Snackbar] Add a maxWidth property #12327

Closed
wants to merge 3 commits into from
Closed

[Snackbar] Add a maxWidth property #12327

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
9973fb0
Add an optional maxWidth parameter to the tooltip component.
Jul 29, 2018
857587e
let's merge
oliviertassinari Jul 29, 2018
5cc5d75
Add button spacing for VariableWidth and CustomizedTooltips demos.
Jul 29, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions docs/src/pages/demos/tooltips/VariableWidth.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import React from 'react';
import Button from '@material-ui/core/Button';
import Tooltip from '@material-ui/core/Tooltip';

const longText = `
Aliquam eget finibus ante, non facilisis lectus. Sed vitae dignissim est, vel aliquam tellus.
Praesent non nunc mollis, fermentum neque at, semper arcu.
Nullam eget est sed sem iaculis gravida eget vitae justo.
`;

function VariableWidth() {
return (
<div>
<Tooltip title={longText}>
<Button>Default Width</Button>
</Tooltip>
<Tooltip title={longText} maxWidth={500}>
<Button>Wraps at 500</Button>
</Tooltip>
<Tooltip title={longText} maxWidth={0}>
<Button>No wrapping</Button>
</Tooltip>
</div>
);
}

export default VariableWidth;
6 changes: 6 additions & 0 deletions docs/src/pages/demos/tooltips/tooltips.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,9 @@ By default disabled elements like `Button` do not trigger user interactions so a
## Customized Tooltips

{{"demo": "pages/demos/tooltips/CustomizedTooltips.js"}}

Copy link
Member

Choose a reason for hiding this comment

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

Any particular reason for the change in order? Where we have customization examples they tend to come last.

Copy link
Member

@oliviertassinari oliviertassinari Jul 29, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@mbrookes Trust is, I'm very hesitant to have this property. From my point of view, the main value of this pull request comes from the default value it's setting: max-width: 300.
Regarding having a dedicated property for that? Well, why not using CSS override?
I have the intuition that people will be more interested in the Customized Tooltips tooltip demo than in the max-width. Benchmarking the other tooltip implementations, I have seen very few having this type of customization. Anyway. I want to add analytics on the live documentation regarding demos usage. I think that we should sort the demos, when possible by their popularity, so overall, people scroll less in the pages to find what they are looking for.

What do you think?

Copy link
Contributor Author

@simoami simoami Jul 29, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@oliviertassinari If it helps, I always scroll to the bottom and love viewing all the demos down to the last one :). Another way to know is to gauge the number of user questions that can be associated with not seeing the demos at the bottom. Do you ever have to say "Scroll down to the last demo example" and how often?

I also inserted the demo example before the last one on purpose, because it pertains to what is now a new standard property. It seemed odd to talk about css customization then document a basic property last. This is a shift from the pattern I'm used to see elsewhere.

Copy link
Member

@oliviertassinari oliviertassinari Jul 29, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

@simoami Oh, Interesting habit, I'm not the only one to start at the bottom then!

## Variable Width

The `Tooltip` wraps long text by default to make it readable. Use the `maxWidth` property to customize the width or suppress it.

{{"demo": "pages/demos/tooltips/VariableWidth.js"}}
1 change: 1 addition & 0 deletions packages/material-ui/src/Tooltip/Tooltip.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ export interface TooltipProps
id?: string;
leaveDelay?: number;
leaveTouchDelay?: number;
minWidth?: number;
onClose?: (event: React.ChangeEvent<{}>) => void;
onOpen?: (event: React.ChangeEvent<{}>) => void;
open?: boolean;
Expand Down
11 changes: 11 additions & 0 deletions packages/material-ui/src/Tooltip/Tooltip.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -277,6 +277,7 @@ class Tooltip extends React.Component {
disableHoverListener,
disableTouchListener,
id,
maxWidth,
open: openProp,
placement,
PopperProps,
Expand Down Expand Up @@ -347,6 +348,9 @@ class Tooltip extends React.Component {
},
classes[`tooltipPlacement${capitalize(placementInner.split('-')[0])}`],
)}
style={{
maxWidth: maxWidth > 0 ? maxWidth : undefined,
}}
>
{title}
</div>
Expand Down Expand Up @@ -404,6 +408,12 @@ Tooltip.propTypes = {
* The number of milliseconds after the user stops touching an element before hiding the tooltip.
*/
leaveTouchDelay: PropTypes.number,
/**
* The maximum width of the tooltip.
* This allows long text to be wrapped into multiple lines for better readability.
* Specify `0` to suppress the width constraint.
*/
maxWidth: PropTypes.number,
/**
* Callback fired when the tooltip requests to be closed.
*
Expand Down Expand Up @@ -469,6 +479,7 @@ Tooltip.defaultProps = {
enterTouchDelay: 1000,
leaveDelay: 0,
leaveTouchDelay: 1500,
maxWidth: 300,
placement: 'bottom',
TransitionComponent: Grow,
};
Expand Down
23 changes: 22 additions & 1 deletion packages/material-ui/src/Tooltip/Tooltip.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ describe('<Tooltip />', () => {
});

describe('prop: title', () => {
it('should display if the title is presetn', () => {
it('should display if the title is present', () => {
const wrapper = shallow(<Tooltip {...defaultProps} open />);
assert.strictEqual(wrapper.find(Popper).props().open, true);
});
Expand All @@ -56,6 +56,27 @@ describe('<Tooltip />', () => {
});
});

describe('prop: maxWidth', () => {
it('should pass the property as style', () => {
const customWidth = 400;
const wrapper = mount(<Tooltip {...defaultProps} maxWidth={customWidth} />);
const children = wrapper.find('span');
children.simulate('mouseEnter');
const style = wrapper.find(`.${classes.tooltip}`).props().style;
assert.strictEqual(style.maxWidth, customWidth);
children.simulate('mouseLeave');
});

it('should allow suppression of maxWidth', () => {
const wrapper = mount(<Tooltip {...defaultProps} maxWidth={0} />);
const children = wrapper.find('span');
children.simulate('mouseEnter');
const style = wrapper.find(`.${classes.tooltip}`).props().style;
assert.strictEqual(typeof style.maxWidth, 'undefined');
children.simulate('mouseLeave');
});
});

it('should respond to external events', () => {
const wrapper = shallow(<Tooltip {...defaultProps} />);
wrapper.instance().childrenRef = document.createElement('div');
Expand Down
1 change: 1 addition & 0 deletions pages/api/tooltip.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ title: Tooltip API
| <span class="prop-name">id</span> | <span class="prop-type">string |   | The relationship between the tooltip and the wrapper component is not clear from the DOM. This property is used with aria-describedby to solve the accessibility issue. If you don't provide this property. It fallback to a random generated id. |
| <span class="prop-name">leaveDelay</span> | <span class="prop-type">number | <span class="prop-default">0</span> | The number of milliseconds to wait before hiding the tooltip. This property won't impact the leave touch delay (`leaveTouchDelay`). |
| <span class="prop-name">leaveTouchDelay</span> | <span class="prop-type">number | <span class="prop-default">1500</span> | The number of milliseconds after the user stops touching an element before hiding the tooltip. |
| <span class="prop-name">maxWidth</span> | <span class="prop-type">number | <span class="prop-default">300</span> | The maximum width of the tooltip. This allows long text to be wrapped into multiple lines for better readability. Specify `0` to suppress the width constraint. |
| <span class="prop-name">onClose</span> | <span class="prop-type">func |   | Callback fired when the tooltip requests to be closed.<br><br>**Signature:**<br>`function(event: object) => void`<br>*event:* The event source of the callback |
| <span class="prop-name">onOpen</span> | <span class="prop-type">func |   | Callback fired when the tooltip requests to be open.<br><br>**Signature:**<br>`function(event: object) => void`<br>*event:* The event source of the callback |
| <span class="prop-name">open</span> | <span class="prop-type">bool |   | If `true`, the tooltip is shown. |
Expand Down
7 changes: 7 additions & 0 deletions pages/demos/tooltips.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,13 @@ module.exports = require('fs')
raw: preval`
module.exports = require('fs')
.readFileSync(require.resolve('docs/src/pages/demos/tooltips/PositionedTooltips'), 'utf8')
`,
},
'pages/demos/tooltips/VariableWidth.js': {
js: require('docs/src/pages/demos/tooltips/VariableWidth').default,
raw: preval`
module.exports = require('fs')
.readFileSync(require.resolve('docs/src/pages/demos/tooltips/VariableWidth'), 'utf8')
`,
},
'pages/demos/tooltips/ControlledTooltips.js': {
Expand Down