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.

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

2321: Added minimum space between navigation tiles and toolbar #2489

Merged
merged 4 commits into from
Dec 8, 2023

Conversation

relentless95
Copy link
Contributor

Short description

there is almost no space between navigation tiles and toolbar when there are only a few languages available for a city.

Proposed changes

Created the variable additionalWidth (35px) in dimensions.tsx to be used inside the styled.aside element found in Layout.tsx to increase the distance between the navigation tiles and toolbar.

Side effects

None

Resolved issues

Fixes: #2321

Copy link
Contributor

@LeandraH LeandraH left a comment

Choose a reason for hiding this comment

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

Woops, that looks like a merge conflict that was committed. Do you know how to fix that?

@relentless95
Copy link
Contributor Author

i think so, but i am not sure where the merge conflict is. can you point it out?

@LeandraH
Copy link
Contributor

LeandraH commented Oct 2, 2023

It's a conflict between the two versions of top. You can tell by the very typical structure that git adds to merge conflicts, which is the <<<<< Branch1 piece before the first version, then the ====== to separate the two versions and then the >>>>>> Branch2 piece. You can remove all of that and just leave your version of top.

Copy link
Contributor

@LeandraH LeandraH left a comment

Choose a reason for hiding this comment

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

I think at a screen width of e.g. 1000px, the toolbar is too low

image

web/src/components/Layout.tsx Show resolved Hide resolved
Copy link
Member

@steffenkleinle steffenkleinle left a comment

Choose a reason for hiding this comment

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

Congrats on your first PR here 🎉 Thanks a lot!

web/src/components/Layout.tsx Show resolved Hide resolved
web/src/constants/dimensions.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@LeandraH LeandraH left a comment

Choose a reason for hiding this comment

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

Looks great, thank you!

web/src/components/Layout.tsx Outdated Show resolved Hide resolved
web/src/components/Layout.tsx Outdated Show resolved Hide resolved
@relentless95 relentless95 force-pushed the 2321-minimum-space-btw-navtiles branch from 6009f65 to db27e53 Compare December 7, 2023 16:32
Copy link
Contributor

@LeandraH LeandraH left a comment

Choose a reason for hiding this comment

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

Looks good, thanks! Tested in Firefox

Copy link
Member

@steffenkleinle steffenkleinle left a comment

Choose a reason for hiding this comment

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

Tested on firefox, thanks!

@steffenkleinle
Copy link
Member

@relentless95 can you run prettier once? Seems like some formatting is still of in Layout.tsx.

@relentless95 relentless95 merged commit 925db3e into main Dec 8, 2023
8 checks passed
@relentless95 relentless95 deleted the 2321-minimum-space-btw-navtiles branch December 8, 2023 11:07
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.

Add minimum space between navigation tiles and toolbar
3 participants