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

Update dropdown on store page #11534

Merged
merged 5 commits into from
Oct 8, 2024
Merged

Update dropdown on store page #11534

merged 5 commits into from
Oct 8, 2024

Conversation

nanaya
Copy link
Collaborator

@nanaya nanaya commented Oct 7, 2024

Some hack involving floating the label is involved 🔧

&__label {
display: flex;
justify-content: space-between;
margin: 0;
color: var(--label-colour);
font-size: @font-size--normal;
padding: var(--label-padding-base) var(--input-padding-base) 0;
padding: 5px var(--input-padding-base) 0;
position: absolute;
Copy link
Collaborator

Choose a reason for hiding this comment

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

can do with a comment on why it's position: absolute, also a note for the sizing of --label-height (or calculate it instead?)

@@ -3,7 +3,9 @@

.input-container {
--label-font-size: @font-size--normal;
--label-height: calc(var(--label-font-size) + var(--input-padding-base) + 5px);
--label-height: calc(
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this should be font-size * 1.25 (the line-height) + padding base tbh 🤔

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

eh close enough 🤷

notbakaneko
notbakaneko previously approved these changes Oct 8, 2024
@nanaya
Copy link
Collaborator Author

nanaya commented Oct 8, 2024

wait no, that was with wrong padding - it should be 5px both sides

@notbakaneko
Copy link
Collaborator

wut

@notbakaneko notbakaneko merged commit 5938c25 into ppy:master Oct 8, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants