Conversation
ravicious
added
no-changelog
ravicious
force-pushed
the
r7s/vnet-info-tab
branch
from
047bcc8 to
ebb5e63
Compare
|
|
||
| /** Returns color of the top bar of DemoTerminal. */ | ||
| export const topBarColor = (theme: DefaultTheme): string => | ||
| emphasize(theme.colors.levels.deep, 0.2); |
There was a problem hiding this comment.
This will be used in a later commit to stack two DemoTerminals on top of each other without creating a visible gap between them (since a DemoTerminal has border radius).
| connected: true, | ||
| leaf: false, | ||
| proxyHost: 'teleport-local:3080', | ||
| proxyHost: 'teleport-local.com:3080', |
There was a problem hiding this comment.
This is purely so that stories reflect the reality more closely. I doubt that the majority of people have a cluster set up as a TLD.
| large: 1280, | ||
| 700: 700, | ||
| 900: 900, | ||
| 1200: 1200, |
There was a problem hiding this comment.
I'll mention this on Slack, but I talked with Rishi and the design team doesn't really use any preset breakpoints – they might differ page to page. However, styled-system requires breakpoints to be defined ahead of time (I think Chakra requires that as well).
So instead of binding them to specific names, I just used numeric values.
There was a problem hiding this comment.
At first I made diagrams for the section with web apps with SSO. However, Zac and I agreed that they don't really help at all. To keep the layout of the page intact, the design team recommended to replace the diagrams with a generic image. But what should this image be?
For now, I just lifted this app access image from Teleport Labs.
| <Box | ||
| // Enough width so that the background repeats just twice at full page width. | ||
| width="66%" | ||
| height="100%" | ||
| css={` | ||
| background: url(${appAccessPng}); | ||
| background-size: contain; | ||
| background-repeat: space no-repeat; | ||
| background-position: center; | ||
| `} | ||
| /> |
There was a problem hiding this comment.
The generic image for this section cannot be allowed to keep its aspect ratio according to available width, as then it's way bigger than the text part of this section which looks weird.
However, if we constrain the height of the image to the height of the text part (by using it as a background image rather than a regular <image>), then on wider sizes it looks too small.
To work around this, I just… made the browser repeat the image. 🤷 It looks alright acceptable for now and we can adjust it later.
Screenshots of alternatives
Aspect ratio is kept
Adjusted to height of text
| 'workspacesService', | ||
| useCallback(state => state.rootClusterUri, []) | ||
| ); | ||
| const isUserInWorkspace = !!rootClusterUri; |
There was a problem hiding this comment.
Assigning this to a variable just so that the ternary in the return value is easier to read.
| * Finds an existing doc using findExisting and opens it. If there's no existing doc, uses | ||
| * createNew to add a new doc and then opens it. | ||
| */ | ||
| openExistingOrAddNew( |
There was a problem hiding this comment.
Finally decided to create a method for that after years of writing this by hand. 😂
gzdunek
left a comment
gzdunek
left a comment
There was a problem hiding this comment.
I don't have any comments, looks great!
|
|
||
| <DemoTerminal | ||
| flex={demoFlex} | ||
| title={userAtHost} |
There was a problem hiding this comment.
This is really a nice touch 👌
| * Finds an existing doc using findExisting and opens it. If there's no existing doc, uses | ||
| * createNew to add a new doc and then opens it. | ||
| */ | ||
| openExistingOrAddNew( |
|
A little bit of empty space between the bottom two terminals would look better IMO. |
| <DemoTerminal title={userAtHost} text={proxyWithoutVnet} /> | ||
| <DemoTerminal title={userAtHost} text={curlWithoutVnet} /> | ||
| </Box> | ||
| <Stack flex={demoFlex} gap={2} fullWidth> |
There was a problem hiding this comment.
Added some space between terminals. Had to add a bit more than I'd like because otherwise the shadows would clash in the light theme.
| maxWidth="1400px" | ||
| width="100%" | ||
| mx="auto" | ||
| p={{ _: 6, 900: 9, 1200: 11 }} |
There was a problem hiding this comment.
i've never seen this usage before. does _: 6 mean default breakpoint?
There was a problem hiding this comment.
Yeah, _ means "no breakpoint". https://github.com/styled-system/styled-system/blob/v5.1.5/docs/responsive-styles.md#using-objects
Breakpoints in Chakra work in a similar way. https://www.chakra-ui.com/docs/theming/customization/breakpoints
I think we haven't used it before because we've been on styled-system 3.1 for so long and this was added only in 4.0. styled-system <4.0 had a much worse solution for breakpoints, so it's no wonder we weren't using it.
|
Merging this now and taking responsibility for fixing any mess related to the new breakpoints once we decide with the design team what to do about them. |
|
@ravicious See the table below for backport results.
|
4 participants
Part of #52788
Figma
This PR adds just the tab. The next PR is going to integrate it with the VNet deep link and first launch of VNet through the Connect button next to a TCP app.