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

Fix accessible namespaces notification not navigating to correct settings #4048

Merged
merged 3 commits into from Oct 15, 2021

Conversation

Nokel81
Copy link
Collaborator

@Nokel81 Nokel81 commented Oct 14, 2021

  • Cleaned up the broadcasting of failure to that the debounding is only
    on renderer. This leads to faster initial notification.

  • Improved logging to not log entire request.Response object

Signed-off-by: Sebastian Malton sebastian@malton.name

fixes #4034

…ings

- Cleaned up the broadcasting of failure to that the debounding is only
  on renderer. This leads to faster initial notification.

- Improved logging to not log entire request.Response object

Signed-off-by: Sebastian Malton <sebastian@malton.name>
@Nokel81 Nokel81 added bug Something isn't working blocker labels Oct 14, 2021
@Nokel81 Nokel81 requested a review from a team as a code owner October 14, 2021 18:57
@Nokel81 Nokel81 requested review from jweak and aleksfront and removed request for a team October 14, 2021 18:57
Signed-off-by: Sebastian Malton <sebastian@malton.name>
@Nokel81
Copy link
Collaborator Author

Nokel81 commented Oct 14, 2021

FYI This doesn't actually fix all the bugs in this because we can no longer scroll into view a sub-setting. I think we should switch this to something similar to what #3573 did from user preferences.

Copy link
Contributor

@aleksfront aleksfront left a comment

Choose a reason for hiding this comment

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

All good instead need to fix Proxy to proxy in cluster-status.ts.

manageProxySettings = () => {
    navigate(entitySettingsURL({
      params: {
        entityId: this.props.clusterId,
      },
      fragment: "proxy",
    }));
  };

Minor but cluster status link doesn't work properly.

Signed-off-by: Sebastian Malton <sebastian@malton.name>
@Nokel81
Copy link
Collaborator Author

Nokel81 commented Oct 15, 2021

@aleksfront Fixed, WDYT about a future where the top level menus are routes?

@aleksfront
Copy link
Contributor

@aleksfront Fixed, WDYT about a future where the top level menus are routes?

Sure it should be done as in Preferences page.

@Nokel81 Nokel81 merged commit 246305c into master Oct 15, 2021
@Nokel81 Nokel81 deleted the issue-4034 branch October 15, 2021 13:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocker bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Clicking on the accessible namespaces button in notification does not go to the correct setting
2 participants