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

Refactor old components #6724

Merged
merged 21 commits into from
Nov 29, 2023
Merged

Refactor old components #6724

merged 21 commits into from
Nov 29, 2023

Conversation

PavelJankoski
Copy link
Contributor

@PavelJankoski PavelJankoski commented Nov 28, 2023

Summary

References #6376

Changes

List of refactored components:

  • /pkg/webui/console/lib/components/with-feature-requirement.js -> removed
  • /pkg/webui/console/containers/gateway-connection/gateway-connection-reactor.js -> removed
  • /pkg/webui/lib/components/error-view.js -> As if now, there is no way to turn functional component into error boundary. Check this thread

Removed every usage of connect, except:

  • pkg/webui/lib/components/full-view-error because useSelector cannot be used outside of redux Provider(there are two usages of that component outside Provider. pkg/webui/console.js and pkg/webui/account.js)
  • pkg/webui/account/containers/collaborators-table and pkg/webui/console/containers/applications-table. Check what's the correct way to refactor when there is merge props used.

Checklist

  • Scope: The referenced issue is addressed, there are no unrelated changes.
  • Compatibility: The changes are backwards compatible with existing API, storage, configuration and CLI, according to the compatibility commitments in README.md for the chosen target branch.
  • Documentation: Relevant documentation is added or updated.
  • The steps/process to test this feature are clearly explained including testing for regressions.
  • Changelog: Significant features, behavior changes, deprecations and fixes are added to CHANGELOG.md.
  • Commits: Commit messages follow guidelines in CONTRIBUTING.md, there are no fixup commits left.

@PavelJankoski PavelJankoski added the ui/web This is related to a web interface label Nov 28, 2023
@PavelJankoski PavelJankoski self-assigned this Nov 28, 2023
@PavelJankoski PavelJankoski changed the title Fix/refactor old components Refactor old components Nov 28, 2023
@PavelJankoski PavelJankoski marked this pull request as ready for review November 28, 2023 14:33
@PavelJankoski PavelJankoski requested review from a team as code owners November 28, 2023 14:33
Copy link
Member

@kschiffer kschiffer left a comment

Choose a reason for hiding this comment

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

お疲れ様でした!

This is quite a large diff, so hard to review in detail. The end-to-end tests are passing though. I've checked it out locally as well and didn't see any warnings or issues. We should do some extra scrutiny once this is on staging.

cypress/e2e/console/integrations/pub-subs/create.spec.js Outdated Show resolved Hide resolved
@kschiffer
Copy link
Member

  • pkg/webui/lib/components/full-view-error because useSelector cannot be used outside of redux Provider(there are two usages of that component outside Provider. pkg/webui/console.js and pkg/webui/account.js)

That's fine.

  • pkg/webui/account/containers/collaborators-table and pkg/webui/console/containers/applications-table. Check what's the correct way to refactor when there is merge props used.

Let's discuss in the frontend meeting.

@PavelJankoski PavelJankoski merged commit eef2d78 into v3.28 Nov 29, 2023
4 of 6 checks passed
@PavelJankoski PavelJankoski deleted the fix/refactor-old-components branch November 29, 2023 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ui/web This is related to a web interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants