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

🐾 Update to PF5 - part II - onClick event handler #1208

Merged
merged 1 commit into from
Jun 19, 2024

Conversation

sgratch
Copy link
Collaborator

@sgratch sgratch commented Jun 10, 2024

Reference: #1098

Include 2 changes:

  1. Seems like onClick event handler parameters were not changed in PF5.
    But to be on the safe side and avoid uncaught PF 4 -> PF 5 migration errors, this PR typed (i.e. (add type to function
    signature) all onClick callback appearances which include the event parameter.
  2. As an enhancement for code readability, named most of onClick callback appearances (when possible).

@yaacov yaacov added the enhancement Categorizes issue or PR as related to a new feature. label Jun 10, 2024
@yaacov yaacov added this to the 2.7.0 milestone Jun 10, 2024
@yaacov yaacov changed the title Update to PF5 - part II - onClick event handler 🐾 Update to PF5 - part II - onClick event handler Jun 10, 2024
@sgratch sgratch force-pushed the onclick-event-handler-naming-typed branch from 071e6ea to fdaa1e7 Compare June 16, 2024 18:21
Copy link

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
21.2% Duplication on New Code

See analysis details on SonarCloud

@sgratch sgratch requested a review from yaacov June 16, 2024 18:48
@sgratch sgratch force-pushed the onclick-event-handler-naming-typed branch from fdaa1e7 to 00c76ce Compare June 17, 2024 15:02
@sgratch sgratch requested a review from yaacov June 17, 2024 16:01
@sgratch sgratch force-pushed the onclick-event-handler-naming-typed branch from 00c76ce to dc9ee17 Compare June 18, 2024 11:27
@sgratch sgratch requested a review from yaacov June 18, 2024 11:42
Reference: kubev2v#1098

Seems like onClick event handler parameters was changed in PF5 for few of the
components by enforcing the event handler as the first parameter (E.g. https://www.patternfly.org/components/chip/#chip
while in our code we are not passing the event
https://github.com/kubev2v/forklift-console-plugin/blob/686daff412d61c5e84c14f7b87fe2867038fc14b/packages/forklift-console-plugin/src/modules/Plans/views/create/components/ChipsToolbarProviders.tsx#L40).

For avoiding uncaught migration errors, this PR named and typed all callback appearances, when possible

Signed-off-by: Sharon Gratch <[email protected]>
@sgratch sgratch force-pushed the onclick-event-handler-naming-typed branch from dc9ee17 to 5c6e273 Compare June 18, 2024 21:01
@yaacov yaacov merged commit 4051689 into kubev2v:main Jun 19, 2024
7 checks passed
@yaacov yaacov modified the milestones: 2.7.0, 2.6.3 Jun 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants