Skip to content

Conversation

@willnorris
Copy link
Member

@willnorris willnorris commented Dec 5, 2023

Add left and right padding around entire client so that the cards don't run into the side of the screen. Also tighten up vertical spacing around header.

There's probably more we could do, like lessen the side padding inside the cards, but I'm just starting with the most glaring issues.

Updates #10261

2023-12-04 03 04 08 2023-12-04 03 03 25

Copy link
Contributor

@sonovawolf sonovawolf left a comment

Choose a reason for hiding this comment

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

LGTM from code, left a couple questions because I don't have a local instance where I can test myself.

<WebClient auth={auth} newSession={newSession} />
)}
</main>
<div className="px-2">
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not put px-2 on the body?

Copy link
Member Author

Choose a reason for hiding this comment

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

no great reason. I was thinking about keeping all of the styling within the App component, but I guess that doesn't really buy us anything. moved to body.

return (
<>
<div className="flex justify-between items-center mb-12">
<div className="flex justify-between items-center mb-9 md:mb-12">
Copy link
Contributor

Choose a reason for hiding this comment

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

Do you need both justify-between and items-center?

Copy link
Member Author

Choose a reason for hiding this comment

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

yes, they control spacing on different axes.

Add left and right padding around entire client so that the cards don't
run into the side of the screen. Also tighten up vertical spacing in
couple of places.

Updates #10261

Signed-off-by: Will Norris <will@tailscale.com>
@willnorris willnorris force-pushed the will/webclient-mobile branch from 81a8496 to 65549d0 Compare December 5, 2023 13:47
@willnorris willnorris self-assigned this Dec 5, 2023
Copy link
Member

@soniaappasamy soniaappasamy left a comment

Choose a reason for hiding this comment

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

Code lgtm

@willnorris willnorris merged commit c5208f8 into main Dec 5, 2023
@willnorris willnorris deleted the will/webclient-mobile branch December 5, 2023 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants