Skip to content
This repository has been archived by the owner on Jun 3, 2024. It is now read-only.

Next Tab Button for dcc.Tabs #786

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ This project adheres to [Semantic Versioning](http://semver.org/).
- node.customdata and link.customdata in `sankey` traces [#4621](https://github.com/plotly/plotly.js/pull/4621)
- `opacityscale` for `surface` traces [#4480](https://github.com/plotly/plotly.js/pull/4480)

### Added
- [#786](https://github.com/plotly/dash-core-components/pull/786) Added 'Next Tab' optional button to dcc.Tabs

## [1.8.1] -2020-02-27
### Added
- [#760](https://github.com/plotly/dash-core-components/pull/760) Added R examples to package help
Expand Down
166 changes: 166 additions & 0 deletions src/components/Tabs.react.js
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,96 @@ EnhancedTab.defaultProps = {
},
};

const NextTab = ({
Copy link
Contributor

Choose a reason for hiding this comment

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

Consider wrapping in React.memo to make this component Pure.
https://reactjs.org/docs/react-api.html#reactmemo

Copy link
Contributor

Choose a reason for hiding this comment

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

This file is becoming very big. Consider breaking up the Tabs component file into multiple files. One way to do this would be to rename src/components/Tabs.react.js to src/components/Tabs.react/index.js (existing imports will recognize <folder>/index.js as the same as <folder>.js and won't require changes) and moving EnhancedTabs and NextTab into files of their own, referenced here.

className,
style,
selectHandler,
value,
disabled,
disabled_style,
disabled_className,
mobile_breakpoint,
amountOfTabs,
colors,
vertical,
loading_state,
title,
}) => {
let tabStyle = style;
if (disabled) {
tabStyle = {tabStyle, ...disabled_style};
}
let tabClassName = `tab ${className || ''}`;
if (disabled) {
tabClassName += ` tab--disabled ${disabled_className || ''}`;
}
return (
<div
data-dash-is-loading={
(loading_state && loading_state.is_loading) || undefined
}
className={tabClassName}
id="next-tab"
Copy link
Contributor

Choose a reason for hiding this comment

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

This component fragment can't have a constant id. If there are multiple dcc.Tabs in the page, they will clash. You could use the component's id and append something to it like ${id}-next-tab.

Copy link
Contributor

Choose a reason for hiding this comment

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

For example, EnhancedTab uses the child component's id, which should not be duplicated.
https://github.com/plotly/dash-core-components/pull/786/files#diff-87a94af645034b5ac219cbbfb96a6295R301

style={tabStyle}
onClick={() => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Use the useCallback hook instead so as to not create a new function for this callback each time the component is rendered.

if (!disabled) {
selectHandler(value);
}
}}
>
<span>{title}</span>
<style jsx>{`
Copy link
Contributor

Choose a reason for hiding this comment

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

.tab {
display: inline-block;
background-color: ${colors.background};
border: 1px solid ${colors.border};
border-bottom: none;
padding: 20px 25px;
transition: background-color, color 200ms;
width: 100%;
text-align: center;
box-sizing: border-box;
}
.tab:last-of-type {
border-right: 1px solid ${colors.border};
border-bottom: 1px solid ${colors.border};
}
.tab:hover {
cursor: pointer;
background-color: #f0f0f0;
}
.tab--selected {
border-top: 2px solid ${colors.primary};
color: black;
background-color: white;
}
.tab--selected:hover {
background-color: white;
}
.tab--disabled {
color: #d6d6d6;
}

@media screen and (min-width: ${mobile_breakpoint}px) {
.tab {
border: 1px solid ${colors.border};
border-right: none;
${vertical
? ''
: `width: calc(100% / ${amountOfTabs});`};
}
.tab--selected,
.tab:last-of-type.tab--selected {
border-bottom: none;
${vertical
? `border-left: 2px solid ${colors.primary};`
: `border-top: 2px solid ${colors.primary};`};
}
}
`}</style>
</div>
);
};
/**
* A Dash component that lets you render pages with tabs - the Tabs component's children
* can be dcc.Tab components, which can hold a label that will be displayed as a tab, and can in turn hold
Expand Down Expand Up @@ -168,6 +258,7 @@ export default class Tabs extends Component {
render() {
let EnhancedTabs;
Copy link
Contributor

Choose a reason for hiding this comment

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

Not a change from this PR, but consider renaming to enhancedTabs - Pascal-Casing for variables deviates from the norm.

let selectedTab;
let nextTab;

if (this.props.children) {
const children = this.parseChildrenToArray();
Expand Down Expand Up @@ -229,6 +320,69 @@ export default class Tabs extends Component {
});
}

if (this.props.children) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nextTab = () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't need to be a function, just create the instance of NextTab here directly

// TODO: handle components that are not dcc.Tab components (throw error)
// enhance Tab components coming from Dash (as dcc.Tab) with methods needed for handling logic
let childProps;
const child = this.parseChildrenToArray()[0];
const length = this.props.children.length;
let currentIndex = 0;

const nextTabHandler = () => {
currentIndex = this.props.value.slice(-1) - 1;
currentIndex = ((currentIndex + 1) % length) + 1;
this.props.setProps({value: `tab-${currentIndex}`});
};

if (
Copy link
Contributor

Choose a reason for hiding this comment

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

// disabled is a defaultProp (so it's always set)
// meaning that if it's not set on child.props, the actual
// props we want are lying a bit deeper - which means they
// are coming from Dash
isNil(child.props.disabled) &&
child.props._dashprivate_layout &&
child.props._dashprivate_layout.props
) {
// props are coming from Dash
childProps = child.props._dashprivate_layout.props;
} else {
// else props are coming from React (Demo.react.js, or Tabs.test.js)
childProps = child.props;
}

return (
<div key={length}>
<NextTab
key={this.props.children.length}
selectHandler={nextTabHandler}
className={childProps.className}
style={
isNil(this.props.next_tab_layout.style)
? childProps.style
: this.props.next_tab_layout.style
}
selectedClassName={childProps.selected_className}
selected_style={childProps.selected_style}
value={childProps.value}
disabled={childProps.disabled}
disabled_style={childProps.disabled_style}
disabled_classname={childProps.disabled_className}
mobile_breakpoint={this.props.mobile_breakpoint}
vertical={this.props.vertical}
colors={this.props.colors}
loading_state={childProps.loading_state}
title={this.props.next_tab_layout.title}
/>
</div>
);
};
}

if (this.props.next_tab) {
Copy link
Contributor

Choose a reason for hiding this comment

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

EnhancedTabs = EnhancedTabs.concat(nextTab());
}

const selectedTabContent = !isNil(selectedTab) ? selectedTab : '';

const tabContainerClass = this.props.vertical
Expand Down Expand Up @@ -326,8 +480,10 @@ Tabs.defaultProps = {
background: '#f9f9f9',
},
vertical: false,
next_tab: false,
persisted_props: ['value'],
persistence_type: 'local',
next_tab_layout: {title: 'Next', style: null},
Copy link
Contributor

@Marc-Andre-Rivet Marc-Andre-Rivet Apr 15, 2020

Choose a reason for hiding this comment

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

next_tab and next_tab_layout could be combined into a single navigation prop that would be extensible in the future. For example:

navigation: PropTypes.object({
  next: PropTypes.oneOf([
    PropTypes.bool,
    PropTypes.object({
      title: PropTypes.string,
      style: PropTypes.object
    })
  ])
})

Would default to false as implemented.

Using true would default to

{ title: 'Next', style: undefined }

Defining title, style would combine/override the defaults above.

Copy link
Contributor

Choose a reason for hiding this comment

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

Later we can add Previous, Last, First, Hide Tabs, etc. to suit our needs.

Copy link
Member

Choose a reason for hiding this comment

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

Instead of title, let's do label so that it matches dcc.Tab (

/**
* The tab's label
*/
label: PropTypes.string,
)

};

Tabs.propTypes = {
Expand Down Expand Up @@ -378,6 +534,16 @@ Tabs.propTypes = {
*/
vertical: PropTypes.bool,

/**
* Adds a next tab button to the component, enabling sequential scrolling through the tabs.
*/
next_tab: PropTypes.bool,

/**
* An object that contains the entries for "title" and "style" to modify the Next Tab button.
*/
next_tab_layout: PropTypes.object,

/**
* Breakpoint at which tabs are rendered full width (can be 0 if you don't want full width tabs on mobile)
*/
Expand Down
40 changes: 40 additions & 0 deletions tests/integration/tab/test_tabs_next_button.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
import pytest
import dash
from dash.dependencies import Input, Output
import dash_core_components as dcc
import dash_html_components as html


@pytest.mark.DCC768
def test_tane001_next(dash_dcc):
app = dash.Dash(__name__)
app.layout = html.Div(
[
html.H1('Dash Tabs Next Button Test'),
dcc.Tabs(
id="tabs-example",
value='tab-1-example',
next_tab=True,
children=[
dcc.Tab(
label='Tab One', value='tab-1', id='tab-1', children="Tab 1 Content"
),
dcc.Tab(
label='Tab Two', value='tab-2', id='tab-2', children="Tab 2 Content"
),
],
),
html.Div(id='tabs-output'),
]
)
@app.callback(Output("tabs-output", "children"), [Input("tabs-example", "value")])
def display_value(value):
return value

dash_dcc.start_server(app)

dash_dcc.wait_for_element('#tab-1').click()

dash_dcc.wait_for_element('#next-tab').click()

dash_dcc.wait_for_text_to_equal("#tabs-output", "tab-2")
Copy link
Contributor

Choose a reason for hiding this comment

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

Would like to see another test case without child tabs that makes sure Next is not present.