fix: edit network wrong urls #5871
Conversation
alter-eggo
force-pushed
the
fix/edit-network-page
branch
from
df33bd4 to
59f59b0
Compare
| > | ||
| Stacks Blockchain API | ||
| </Link>{' '} | ||
| <Box display="inline-block"> |
There was a problem hiding this comment.
this is fix for weird ui error, same elements, but one without underline
wonder if there is some better way to fix it
There was a problem hiding this comment.
That is a weird issue. I spent a few minutes trying different ways but didn't come up with anything quickly.
I think it's related to us using <Link inside of the <span. We could probably refactor this into it's own component and spot the issue faster as it's a bit weird anyway now.
Removing the Box and the first <Link seems to take up loads of space:
It would be good to add a better solution but what you have done is working already so it could be fine
| @@ -37,6 +38,7 @@ export function NetworkItemMenu({ onClickDeleteNetwork, onEditNetwork }: Props) | |||
| </HStack> | |||
| </DropdownMenu.Item> | |||
| <DropdownMenu.Item | |||
| data-testid={NetworkSelectors.DeleteNetworkMenuBtn} | |||
There was a problem hiding this comment.
Is this UI right? Red text and black icon?
There was a problem hiding this comment.
hm, yeah, icon should be red
| return ( | ||
| <> | ||
| <PageHeader title="Add network" /> | ||
| <PageHeader title={title} /> |
There was a problem hiding this comment.
Maybe we can create a different .layout file for add-network and edit-network?
Or else pass a callback to set title to <AddNetworkForm?
I'm not sure why we are using location state for isEditNetworkMode
There was a problem hiding this comment.
Maybe we can create a different .layout file for add-network and edit-network?
hm, is it ok to create .layout components which contains forms? as I understand we use them for layout only, they shouldn't contain any specific logic inside?
Or else pass a callback to set title to <AddNetworkForm?
we use title not in AddNetworkForm, but in AddNetwork which is separate page
I'm not sure why we are using location state for isEditNetworkMode
we pass network data (which we edit) in state already, I thought there is no problem to pass isEditNetworkMode as well, just to be explicit
There was a problem hiding this comment.
We can use a wrapper layout for the different pages and pass the form as children.
We could also use a different Route URL if we wanted for edit-network
We could set the title in AddNetwork by passing a callback to <AddNetworkForm then we don't need to use route state
There was a problem hiding this comment.
We could set the title in AddNetwork by passing a callback to <AddNetworkForm then we don't need to use route state
don't understand tbh how passing callback to AddNetworkForm can help in setting title in AddNetwork
we actually need to pass route state mainly to get network data that we want to edit
There was a problem hiding this comment.
created sep pages for add/edit network and common layout component for them, removed isEditNetworkMode from route state
markmhendrickson
requested review from
pete-watters and
camerow
and removed request for
kyranjamie and
fbwoolf
alter-eggo
force-pushed
the
fix/edit-network-page
branch
from
59f59b0 to
2eff833
Compare
alter-eggo
force-pushed
the
fix/edit-network-page
branch
from
2eff833 to
e10bdd3
Compare
alter-eggo
force-pushed
the
fix/edit-network-page
branch
from
e10bdd3 to
8be00ee
Compare
This pr fixes bug with wrong network urls in edit network form