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

sketch of solution to fix #129 #130

Closed
wants to merge 1 commit into from
Closed

sketch of solution to fix #129 #130

wants to merge 1 commit into from

Conversation

chriddyp
Copy link
Member

@chriddyp chriddyp commented Dec 18, 2017

Here is a sketch of a solution to fix #129. I have not tested this or verified that it worked, it is just a starting point for a community member to pick up.

In order for a PR to be accepted, we'll need to:

If anyone would like to pick up this PR, feel free.

@ned2
Copy link
Contributor

ned2 commented Dec 26, 2017

Well that's a pretty good start! I've verified that the solution works in all major browser with the exception of two. For Safari, I still need to track down an Apple machine, but can do shortly.

IE, on the other hand, is interesting. It turns out that even before this modification, dcc.Link did not actually work. I believe that the dcc.Link has never worked in IE, as it does not support the Event constructor. Just noticed the existing issue #113 about this and have added a comment there.

I'm not really sure what's needed for an integration test. Would you be able to provide some guidance as what would be required? Would this be a separate test independent to test_gallery?

@chriddyp
Copy link
Member Author

chriddyp commented Jan 8, 2018

IE, on the other hand, is interesting. It turns out that even before this modification, dcc.Link did not actually work. I believe that the dcc.Link has never worked in IE, as it does not support the Event constructor. Just noticed the existing issue #113 about this and have added a comment there.

Thanks for reporting! Fix in #140

Would you be able to provide some guidance as what would be required? Would this be a separate test independent to test_gallery?

Gladly! It would be a separate test. The main thing that I'd want to test is that clicking on dcc.Link won't refresh the page - that is, it will act as dcc.Link currently works, without refreshing the page - that is, that dcc.Link caused the update to happen, not the html.A.
One way to test this might be to set app.layout to be a function and assert that it is only fired once, on page start. If it were to fire multiple times, then that would mean that the page was being refreshed and that the click event is firing the html.A instead of the dcc.Link.

Most of the integration tests are in https://github.com/plotly/dash-renderer/blob/master/tests/test_render.py - that's a good place to reference. In particular, we'll probably need to use the "call_count" logic - a multi-processing-safe value that gets incremented when a function gets called:
https://github.com/plotly/dash-renderer/blob/cef2313478ee2c37b91b9ef773bf6991bc96240e/tests/test_render.py#L433-L440

Here's a sketch of what this particular test might look like (I haven't tested this). I'm using some new functions (wait_for_element_by_css_selector, wait_for_text_to_equal) that were introduced in #140 .

    def test_update_link_click(self):
        app = dash.Dash(__name__)
        def serve_layout():
            return html.Div([
                html.Div(id='waitfor'),
                dcc.Location(id='location')
                dcc.Link('Link', href='/test', id='link')
                html.Div(id='output')
            ])
        app.layout = serve_layout
        call_count = Value('i', 0)
        
        @app.callback(Output('output', 'children'), [Input('link', 'href')])
        def update_callback(href):
            call_count.value = call_count.value + 1
            return href

        self.startServer(app=app)
        self.wait_for_element_by_css_selector('#waitfor')
        self.wait_for_text_to_equal('#output', '/')

        self.assertEqual(call_count.value, 1)
        self.snapshot('link - before click')
        self.wait_for_element_by_css_selector('#link').click()
        self.wait_for_text_to_equal('#output', '/test')
        self.assertEqual(call_count.value, 1)
        self.snapshot('link - after click')

To run this test, you can do

$ python -m unittest test.test_integration.Tests.test_update_link_click

To debug this test, I recommend putting in a

import ipdb; ipdb.set_trace()

inside your test. That'll open up the python debugger allowing you to execute commands in your python console. As you execute selenium commands (like self.driver.find_element_by_id('wait_for')), they'll get run in the browser window that you have open. It's like magic 🐰 🎩 🙂

@valentijnnieman
Copy link
Contributor

Closing this because #129 is now closed

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Difficulty emulating vanilla anchor tags with dcc.Link
3 participants