-
Notifications
You must be signed in to change notification settings - Fork 284
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 #37476 - update ak results in hostgroup #10998
Conversation
Can't really speak to what goes on in the code, but it seems to resolve the issue |
@@ -23,24 +23,26 @@ import { selectAPIStatus } from 'foremanReact/redux/API/APISelectors'; | |||
const ActivationKeysSearch = () => { | |||
const ACTIVATION_KEYS = 'ACTIVATION_KEYS'; | |||
const KT_AK_LABEL = 'kt_activation_keys'; | |||
const selectedEnvId = useMemo(() => { | |||
const getSelectedEnvId = () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This seems to be vanilla JS, can we move it outside the component so the function isn't recreated on each render? That may provide the same benefit that the useMemo was supposed to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, thats a better place for it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works well, just one more comment below
return dataId; | ||
}, []); | ||
const [selectedEnvId, setSelectedEnvId] = useState(getSelectedEnvId()); | ||
const [selectedContentViewId, setSelectedContentViewId] = useState(getSelectedContentViewId); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
const [selectedContentViewId, setSelectedContentViewId] = useState(getSelectedContentViewId); | |
const [selectedContentViewId, setSelectedContentViewId] = useState(getSelectedContentViewId()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed, thanks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @MariaAga! ACK
selecting lifecycle env does not update the ak results in new host group, since useMemo doesnt update.
placeholder prop was wrong and should be placeholderText to show empty results