Deprecate LogoutButton #2899
Deprecate LogoutButton #2899
Conversation
dash/_validate.py
Outdated
| @@ -422,6 +423,15 @@ def validate_layout(layout, layout_value): | |||
| component_ids = set() | |||
|
|
|||
| def _validate(value): | |||
| def _validate_type(comp): | |||
| deprecated_types = ["LogoutButton"] | |||
There was a problem hiding this comment.
Just to make sure there is no collision, we should also check the _namespace attribute. Also could add a custom deprecation message to the component.
I would refactor the deprecated_types to a collections.defaultdict(lambda: collections.defaultdict(dict)) then set:
deprecated_components["dash_core_components"]["LogoutButton"] = "''
The Logout Button is no longer used with Dash Enterprise and can be replaced with a html.Button or html.A.
eg: html.A(href=os.getenv('DASH_LOGOUT_URL'))
"""
The for the check you could have:
_type = getattr(comp, "_type", "")
_ns = getattr(comp, "_namespace", "")
deprecation_message = deprecated_components[_ns][_type]
if deprecation_message:
...
There was a problem hiding this comment.
Changed in 87385a9
- Checks for namespace on top of the type - Modifies warning message - Modifies test to capture new warning message
| dash_dcc.start_server(app) | ||
| time.sleep(1) | ||
| dash_dcc.percy_snapshot("Core Logout button") | ||
| with pytest.warns( |
There was a problem hiding this comment.
Need to add a condition if add_initial_logout_button for testing the deprecation notice otherwise it fails. Maybe better to just test the deprecation notice in a new test.
There was a problem hiding this comment.
Right. My worry is that DeprecationWarning is not being output when the LogoutButton is being used inside a callback—only when the LogoutButton is defined in app.layout.
This is the reason I was checking for the warning in the two separate scenarios. However, if we don't need to output a DeprecationWarning when the LogoutButton is used inside a callback, then I can just create a new test.
There was a problem hiding this comment.
@leeagustin can you please let us know if you added or are interested in adding the new test? thanks - @gvwilson
gvwilson
added
feature
|
Merging into branch |
Fixes #2894.
PR will output a
DeprecationWarningwhen theLogoutButtonis used.I tried making the changes from #2894 (comment), but it seems that it only validates the initial page layout and not the components used in callbacks.
As seen in the modified test in this PR, the test doesn't output a
DeprecationWarningand fails when the LogoutButton is in the callback but not in the initial page layout. On the other hand, it passes whenLogoutButtonis in the initial page layout.I'm assuming we would also want to show a
DeprecationWarningifLogoutButtonis used in a callback. What would be a good way to approach this? Or would it be simpler to remove theLogoutButtoncomponent entirely?Contributor Checklist
optionals
CHANGELOG.md