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

Fixes #37746 - Update to pf5 #10272

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from
Open

Conversation

MariaAga
Copy link
Member

@MariaAga MariaAga commented Aug 13, 2024

Needs theforeman/foreman-js#481 & to remove pf5 from package.json
Will need plugin updates as well.

package.json Outdated Show resolved Hide resolved
Copy link
Contributor

@kmalyjur kmalyjur left a comment

Choose a reason for hiding this comment

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

Except for a few small things I commented on (and some not successful checks), everything looks fine, thank you! 🥳

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe the first comment could be updated?

app/assets/stylesheets/patternfly_colors_overrides.scss Outdated Show resolved Hide resolved
Comment on lines 61 to 62
hasNoOffset: false,
className: undefined,
Copy link
Contributor

Choose a reason for hiding this comment

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

  • hasNoOffset is false by default
  • is the className necessary?

Comment on lines +90 to +104
return [
isDeleteable && {
title: __('Delete'),
onClick: () => onDeleteClick({ id, name }),
isDisabled: !canDelete,
isAriaDisabled: !canDelete,
},
...((getActions &&
getActions({ id, name, canDelete, canEdit, ...item })) ??
[]),
...extendActions.map(action => ({
...action,
isAriaDisabled: action.isDisabled,
})),
].filter(Boolean);
};
Copy link
Contributor

Choose a reason for hiding this comment

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

The functionality is fine, however, when opening the table kebab, the "Edit" button looks like it's selected and the mouse on the "Change content..." looks like it's clickable:
image

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

Successfully merging this pull request may close these issues.

2 participants