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

Tabs and Tab component bug-fixes #267

Merged
merged 13 commits into from Aug 22, 2018

Conversation

Projects
None yet
5 participants
@valentijnnieman
Copy link
Contributor

commented Aug 17, 2018

This fixes the bug where the Tabs component breaks if no children props are set. This should fix #265 too.

It also fixes the bug where the 2nd tab was selected by default - #262

(Sorry, I had some trouble with my commits, it looks a bit wonky)

valentijnnieman added some commits Aug 17, 2018

@valentijnnieman

This comment has been minimized.

Copy link
Contributor Author

commented Aug 17, 2018

It looks like this is the issue with the committed bundle files: https://unix.stackexchange.com/questions/32001/what-is-m-and-how-do-i-get-rid-of-it

@valentijnnieman valentijnnieman changed the title Prevent Tabs from breaking if there are no props.children set on it [WIP] Tabs and Tab component bug-fixes Aug 17, 2018

@valentijnnieman valentijnnieman changed the title [WIP] Tabs and Tab component bug-fixes Tabs and Tab component bug-fixes Aug 17, 2018

@radekwlsk

This comment has been minimized.

Copy link

commented Aug 20, 2018

Any ETA on new version merging those fixes?

@T4rk1n
Copy link
Contributor

left a comment

Should have rebased instead of regenerating all the components. Fix the indentation.

@@ -124,53 +124,62 @@ export default class Tabs extends Component {
}
render() {

This comment has been minimized.

Copy link
@T4rk1n

T4rk1n Aug 22, 2018

Contributor

The indentation is 2 space while the rest of the components is 4 spaces.

@@ -456,6 +456,20 @@ def render_content(tab):
time.sleep(2)
self.assertEqual(tabs_content.text, 'Test content 1')


This comment has been minimized.

Copy link
@T4rk1n

T4rk1n Aug 22, 2018

Contributor

⚡️ Only one space between methods in classes.

@@ -456,6 +456,20 @@ def render_content(tab):
time.sleep(2)
self.assertEqual(tabs_content.text, 'Test content 1')


def test_tabs_with_children_undefined(self):

This comment has been minimized.

Copy link
@T4rk1n

T4rk1n Aug 22, 2018

Contributor

🐅 I think this test could be improved, right now it doesn't do much. Maybe add the tabs in a callback triggered by a button ?

This comment has been minimized.

Copy link
@valentijnnieman

valentijnnieman Aug 22, 2018

Author Contributor

This test tests if a Tabs component still renders even if children is undefined. Previously, if children was not set, it would break, thus throwing an error, and thus failing this test. This PR fixes #265 as mentioned above.


const amountOfTabs = this.props.children.length
const amountOfTabs = this.props.children.length

This comment has been minimized.

Copy link
@T4rk1n

T4rk1n Aug 22, 2018

Contributor

Missing ;

This comment has been minimized.

Copy link
@wbrgss

wbrgss Aug 22, 2018

Member

+1, technically we should be using semicolons.

Many more missing -- too many to mention in a review. I recommend adding
/*eslint semi: ["error", "always"]*/
to the top of this file and linting again to catch them

@valentijnnieman

This comment has been minimized.

Copy link
Contributor Author

commented Aug 22, 2018

@T4rk1n What good would rebasing have done in this case? I'm seeing a change in end-of-line characters, when I rebuild the Tabs component it automatically rebuild all of the other components because the end-of-line characters where different (see link https://unix.stackexchange.com/questions/32001/what-is-m-and-how-do-i-get-rid-of-it)

@T4rk1n

This comment has been minimized.

Copy link
Contributor

commented Aug 22, 2018

@valentijnnieman Your right. it put the ^M in the docstrings, probably from when I generated the classes, git doesn't replace the newline in strings automatically.

@wbrgss
Copy link
Member

left a comment

Recommending some more style changes 🐄

Repeating my 'outdated' comment:

Technically we should be using semicolons.
Many more missing -- too many to mention in a review. I recommend adding
/*eslint semi: ["error", "always"]*/
to the top of this file and linting again to catch them


const amountOfTabs = this.props.children.length
const amountOfTabs = this.props.children.length

This comment has been minimized.

Copy link
@wbrgss

wbrgss Aug 22, 2018

Member

+1, technically we should be using semicolons.

Many more missing -- too many to mention in a review. I recommend adding
/*eslint semi: ["error", "always"]*/
to the top of this file and linting again to catch them

this.props.children = [this.props.children];
}
if(this.props.children) {
if (!Array.isArray(this.props.children)) {

This comment has been minimized.

Copy link
@wbrgss

wbrgss Aug 22, 2018

Member

Some inconsistency here (R.is(Array, foo) is used above here). Apparently there's also the undocumented R.isArray.

@valentijnnieman

This comment has been minimized.

Copy link
Contributor Author

commented Aug 22, 2018

@wbrgss That's why we have .eslintrc :-) we'll update the tooling eventually to auto-format, (#274) but for now I don't want to spend time chasing down missing semi-colons and preventing these important fixes from being released. I'm pretty sure that the latest changes (where I ran Prettier) already caught everything by the way!

@wbrgss

wbrgss approved these changes Aug 22, 2018

Copy link
Member

left a comment

💃 from me

@T4rk1n

This comment has been minimized.

Copy link
Contributor

commented Aug 22, 2018

💃

@valentijnnieman valentijnnieman merged commit 26b2c62 into master Aug 22, 2018

2 checks passed

ci/circleci Your tests passed on CircleCI!
Details
percy/dash-core-components No visual changes since last approval
Details

@valentijnnieman valentijnnieman deleted the 265-tabs-without-children branch Aug 22, 2018

@renanxcortes

This comment has been minimized.

Copy link

commented Sep 7, 2018

Hi! I deployed a Dash app some weeks ago and everything went okay and worked properly with the tabs, but now I ran the exactly the same code, but now I have to change the tabs whenever I want to update the callback. Do you know what might be happening? Does this PR was supposed to fix this? I`ve looked on #265 also but it didn't solve :(

My layout code is:

app.layout = html.Div(
        html.Div([    
        html.Div([
                    html.H1(children='Web STARS', style={'textAlign': 'center'}),
                    html.H2(children='A web-based version of Space-Time Analysis of Regional Systems',
                            style={'textAlign': 'center'}),
                
                
                dcc.Tabs(id="tabs", style = {'textAlign': 'center', 
                                     'backgroundColor':'lightblue',
                                     #'width': '75%'
                                     },
                children=[
        
                dcc.Tab(label='Presentation', style={'font-weight': 'bold', 'font-size': '120%'}, children=[
                        
                html.H3(children='This is a beta-version of some functions of STARS using Dash from plotly.', 
                style={'textAlign': 'center',
                   'margin-top': '60', 
                   'font-size': '110%'}),
    
                html.Img(src='data:image/png;base64,{}'.format(encoded_image_stars.decode()), 
                 style={'width': '150px',
                'margin-left': 700}),
                
                html.Img(src='data:image/png;base64,{}'.format(encoded_image_plotly.decode()), 
                 style={'width': '150px',
                'margin-left': 700}) 
                        
                        ]),
        
        
                dcc.Tab(label='Spatiotemporal Overview', style={'font-weight': 'bold', 'font-size': '120%'}, children=[
        
#                html.Div([                        
#                        dcc.Checklist(
#                            id='auto-check',
#                            options=[{'label': ' Time Travel Animation ', 'value': 'auto'}],
#                            values=[],
#                        ),
#                        ], style={'padding-top': '200px', 'display': 'inline-block', 'float': 'left'}),
#                
                html.Div([
                        html.P('Select the Year:', style={'margin-top': '5', 
                                                          'font-size': '150%', 
                                                          'font-weight': 'bold',
                                                          'textAlign': 'center'}),
                        dcc.Slider(
                            id='years-slider',
                            min=min(years),
                            max=max(years),
                            value=min(years),
                            marks={str(year): str(year) for year in years_by_step}
                        ),                        
                        dcc.Interval(
                            id='interval-event',
                            interval=24*60*60*1000,
                            n_intervals=0
                        ),
                    
                html.P('Select the Variable to analyze:', style={'margin-top': '35', 'font-size': '150%', 'font-weight': 'bold'}),    
                dcc.RadioItems(
                            id='type_data_selector',
                            options=[
                                {'label': 'Per Capita Relative Ratio (PCR)', 'value': 'pcr'},
                                {'label': 'Raw Income Data', 'value': 'raw'}
                            ],
                            value='pcr',

                            style={'margin-top': '0', 'font-size': '125%'}
                    ),
                    
               html.Div([ 
               html.P('Animation:', style={'margin-top': '20', 'font-size': '150%', 'font-weight': 'bold'})
               ]),
               
               html.Div([
               dcc.Checklist(
                            id='auto-check',
                            options=[{'label': ' Time Travelling ', 'value': 'auto'}],
                            values=[],
                        )]),  
                        
                
                html.Div([
                dcc.Checklist(
                id='spatial_travel-check',
                options=[{'label': ' Spatial Travelling ', 'value': 'auto'}],
                values=[],
                )]),
                
                dcc.Interval(
                    id='spatial_interval-event',
                    interval=24*60*60*1000,
                    n_intervals=0
                ),
                                
                        ], style={'width':1350, 'margin':25, 'float': 'left'}),
                
                html.Div([            
            html.Div([
                        
                            dcc.Graph(
                                id='choropleth-graph'
                            ),
                        
                            dcc.Graph( # ?dcc.Graph to learn more about the properties
                                id='timeseries-graph',
                            clear_on_unhover = 'True' # Sets the slider year when the mouse hover if off the graph
                            ),    
                        
                            # dcc.Graph(
                            #    id='timepath-graph'
                            #),
                                    
                    ], className="four columns"),        
            html.Div([                
                        
                        dcc.Graph(
                                id='scatter-graph'
                            ),
                        
                            html.P('Change the years below:'),
                                
                            dcc.Graph(
                                id='density-graph' 
                            ),
                                               
                            html.P('Choose a pair of years for densities:', style = {'font-weight': 'bold'}),
                            
                            html.Div([
                            dcc.Dropdown(
                                id='initial_years_dropdown',
                                options=years_options,
                                value='1929'
                            ),
                            dcc.Dropdown(
                                id='final_years_dropdown',
                                options=years_options,
                                value='2009'
                            )], className="row"),
                                
                            #   dcc.Graph(
                            #    id='boxplot-graph'
                            #)            
                        
                    ], className="four columns"),        
            html.Div([
                            dcc.Graph(
                                id='timepath-graph'
                            ),
                             dcc.Graph(
                                id='boxplot-graph'
                            ),           
					 ], className="four columns"),    
        ], className="row", style={'width': '100%', 'display': 'inline-block', 'float': 'center'}) ]),
        
        
        dcc.Tab(label='Rank Paths', style={'font-weight': 'bold', 'font-size': '120%'}, children=[
                
                
            html.Div([
            html.P('Select the Rank for the Path:', style = {'font-size': '150%', 'margin-top':25, 'font-weight': 'bold'}),
            
            html.Div([
            dcc.Dropdown(
                            id='rankpath_dropdown',
                            options=ranks_options,
                            value='1'
                        )], style = {'margin-left':600,
                                     'margin-right':600,
                                     'margin-top':10}),
            
            html.Div([
                html.P('Year Highlighted:', style={'margin-top': '25', 'font-size': '150%', 'font-weight': 'bold'}), 
                dcc.Slider(
                            id='years-slider-rank-path',
                            min=min(years),
                            max=max(years),
                            value=min(years),
                            marks={str(year): str(year) for year in years_by_step}
                        )
                ], style={'width': '75%', 
                          'margin-left':50,
                          'margin-right':50,
                          'textAlign': 'center',
                          'display':'inline-block'}),
                    
            html.Div([                           
            dcc.Graph(
                                id='rank-path-graph'
                                
                            )], style = {'margin-left':185,
                                         'margin-right':150,
                                         'margin-top':25})
                    

            
        ], className="twelve columns", style={#'width': '75%',
                                              'textAlign': 'center', 
                                              #'margin-left':150,
                                              #'margin-top':20,
                                              'backgroundColor':'rgb(244, 244, 255)'}) # Or 'lightgray' or look here https://www.w3schools.com/colors/colors_rgb.asp 
                
                
                ]),
        
        
        dcc.Tab(label='Markov and Spatial Markov', style={'font-weight': 'bold', 'font-size': '120%'}, children=[
                
                html.P('Put here the markov features')
                
                ])
        
        
        
        ])
        ])
    ], className='ten columns offset-by-one', 
       style = {'backgroundColor':'rgb(244, 244, 255)'})
)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.