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.

Sign up for GitHub

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

Add basic support for target port in gateways in Connect #50912

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
+874 −226
Open

Add basic support for target port in gateways in Connect #50912

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
d278306
Update type for targetSubresourceName on DocumentGateway
ravicious Jan 2, 2025
83d9248
Use "local port" instead of "port" in DocumentGatewapApp
ravicious Jan 2, 2025
615aeec
Rewrite gateway FieldInputs to use styled components
ravicious Jan 2, 2025
99f2a3a
Update comments in protos
ravicious Jan 2, 2025
0762ce7
useGateway: Stabilize useAsync functions of ports
ravicious Jan 2, 2025
b34d93b
Add padding to menu label if it's first child
ravicious Jan 2, 2025
d6617c3
Add support for required prop to Input and FieldInput
ravicious Jan 2, 2025
e9bcf63
Add UI for changing target port
ravicious Jan 2, 2025
00665a3
ActionButtons: Show ports of multi-port apps when VNet is not supported
ravicious Jan 2, 2025
7d359a5
Add RWMutex to gateways
ravicious Jan 2, 2025
819ecbb
Clear app gateway cert on target port change
ravicious Jan 2, 2025
596b62d
Remove gateways/app.LocalProxyURL
ravicious Jan 2, 2025
53b1061
TestTCP: Close connections when test ends
ravicious Jan 2, 2025
34f5c85
Create context with timeout in testGatewayCertRenewal
ravicious Jan 2, 2025
d3c8122
Add tests for changing the target port of a TCP gateway
ravicious Jan 2, 2025
415cd92
Parallelize app gateway tests within MFA/non-MFA groups
ravicious Jan 2, 2025
4b3cb94
Make testGatewayConnection take ctx as first arg
ravicious Jan 2, 2025
41646e1
Validate target port of app gateways
ravicious Jan 2, 2025
edacb8b
Increase timeout in testGatewayCertRenewal
ravicious Jan 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions web/packages/design/src/Menu/Menu.story.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,18 @@ export const MenuItems = () => (
<MenuItem>Amet nisi tempor</MenuItem>
</OpenMenu>
</Flex>
<Flex gap={3} flexDirection="column">
<H3>Label as first child</H3>
<OpenMenu>
<MenuItemSectionLabel>Tempus ut libero</MenuItemSectionLabel>
<MenuItem>Lorem ipsum</MenuItem>
<MenuItem>Dolor sit amet</MenuItem>
<MenuItemSectionSeparator />
<MenuItemSectionLabel>Leo vitae arcu</MenuItemSectionLabel>
<MenuItem>Donec volutpat</MenuItem>
<MenuItem>Mauris sit</MenuItem>
</OpenMenu>
</Flex>
</Flex>
);

Expand Down
30 changes: 16 additions & 14 deletions web/packages/design/src/Menu/MenuItem.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,37 +71,39 @@ const MenuItemBase = styled(Flex)`
${fromThemeBase}
`;

export const MenuItemSectionLabel = styled(MenuItemBase).attrs({
px: 2,
export const MenuItemSectionSeparator = styled.hr.attrs({
onClick: event => {
// Make sure that clicks on this element don't trigger onClick set on MenuList.
event.stopPropagation();
},
})`
font-weight: bold;
min-height: 16px;
background: ${props => props.theme.colors.interactive.tonal.neutral[1]};
height: 1px;
border: 0;
font-size: 0;
`;

export const MenuItemSectionSeparator = styled.hr.attrs({
export const MenuItemSectionLabel = styled(MenuItemBase).attrs({
px: 2,
onClick: event => {
// Make sure that clicks on this element don't trigger onClick set on MenuList.
event.stopPropagation();
},
})`
background: ${props => props.theme.colors.interactive.tonal.neutral[1]};
height: 1px;
border: 0;
font-size: 0;
font-weight: bold;
min-height: 16px;

// Add padding to the label for extra visual space, but only when it follows a separator.
// If a separator follows a MenuItem, there's already enough visual space, so no extra space is
// needed. The hover state of MenuItem highlights everything right from the separator start to the
// end of MenuItem.
// Add padding to the label for extra visual space, but only when it follows a separator or is the
// first child.
//
// If a separator follows a MenuItem, there's already enough visual space between MenuItem and
// separator, so no extra space is needed. The hover state of MenuItem highlights everything right
// from the separator start to the end of MenuItem.
//
// Padding is used instead of margin here on purpose, so that there's no empty transparent space
// between Separator and Label – otherwise clicking on that space would count as a click on
// MenuList and not trigger onClick set on Separator or Label.
& + ${MenuItemSectionLabel} {
${MenuItemSectionSeparator} + &, &:first-child {
Copy link
Member Author

Choose a reason for hiding this comment

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

This diff is overall confusing since I had to reorder the definitions and then I moved adding that extra padding-top between components. The previous selector for extra padding was also confusing, because it was defined in MenuItemSectionSeparator, but it looked like this: & + ${MenuItemSectionLabel} {. So even though we were in styles for MenuItemSectionSeparator, we were setting styles for MenuItemSectionLabel when it directly follows a separator. 🙃

It's might be easier to look at the master version and the version from this branch.

padding-top: ${props => props.theme.space[1]}px;
}
`;
Expand Down