Deregister hosts list frontend #1601
Conversation
arbulu89
force-pushed
the
deregister-hosts-list-frontend
branch
2 times, most recently
from
7acda32
to
2ed1ec4
Compare
arbulu89
force-pushed
the
deregister-hosts-list-frontend
branch
from
2ed1ec4
to
98083f2
Compare
arbulu89
force-pushed
the
deregister-hosts-list-frontend
branch
from
98083f2
to
621fa21
Compare
arbulu89
force-pushed
the
deregister-hosts-list-frontend
branch
from
621fa21
to
7be678b
Compare
| return host; | ||
| }); | ||
| }, | ||
| setHostNotDeregistering: (state, action) => { |
There was a problem hiding this comment.
I have a doubt about naming. I would have thought of something that talks about starting a deregistration and completing a deregistration.
No hard feelings tho 😄
assets/js/components/HostsList.jsx
Outdated
| @@ -50,6 +53,8 @@ function HostsList() { | |||
| ); | |||
|
|
|||
| const [searchParams, setSearchParams] = useSearchParams(); | |||
| const [cleanUpModalOpen, setCleanUpModalOpen] = useState(false); | |||
| const [selectedHost, setSelectedHost] = useState(undefined); | |||
There was a problem hiding this comment.
Maybe we can rename the state property to hostToDeregister or something like that, selectedHost seems to generic
| content && ( | ||
| <CleanUpButton | ||
| cleaning={item.deregistering} | ||
| onClick={() => { |
There was a problem hiding this comment.
Can we extract this to a named function in the component?
There was a problem hiding this comment.
I'm not a big fan of creating functions for such trivial things (2 lines in fact), as it forces the user to scroll up and down in the code, to find out such a simple thing.
Anyway, for the sake of a request and thumbs up, i will apply the change hehe
There was a problem hiding this comment.
@CDimonaco This is how it looks like now: 3614c2e
Personally, I find it uglier (specially, because the functions are still inside the component). What do you think?
| <DeregistrationModal | ||
| hostname={selectedHost?.hostname} | ||
| isOpen={!!cleanUpModalOpen} | ||
| onCleanUp={() => { |
There was a problem hiding this comment.
Can we extract this to a named function in the component?
|
lgtm |
Description
Add deregistration button and modal.
It includes a simple saga to handle this.
We need to add the same button in the host details view, with the unique different that when the call is successful, we should navigate to the host list.
Blocked by
#1599
#1587
How was this tested?
Tested