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

Fix: Not visible imagery names + input proportion #8975

Merged
merged 3 commits into from
Jun 6, 2022

Conversation

furkanmutlu
Copy link
Contributor

This PR fixes not visible background imagery names and not proportional radio buttons for backgrounds panel.

@tordans
Copy link
Collaborator

tordans commented Feb 10, 2022

Hi! The way I read the CSS is, that the whole point of the span-tag is to add the css that prevents the line from breaking. Instead of adding more css on top that re-writes this, I would be cleaner to not use the span here or refactor the span into two <span class="one-line">… vs. <span class="multiline">… (or similar).

For the multiline, I suggest to also add something like line-height: 0.95rem; to fix the line-spacing.


Also: Adding a screenshot of the new look makes it easier to understand most PRs.

@furkanmutlu
Copy link
Contributor Author

furkanmutlu commented Feb 10, 2022

Hi @tordans, thank you for your feedback. I think refactoring for two types of spans (one-line / multi-line) would be overkill for this issue. I found simpler solution by removing white-space, text-overflow and flex grow from the span. And set new width for it by removing paddings around to ensure radio input size doesn't change. Also added line-height as you mentioned. Please find the screen shot of new look below:
image

@furkanmutlu
Copy link
Contributor Author

@tordans Could you please take a look at this when you have time ? Thanks

@tordans
Copy link
Collaborator

tordans commented Mar 19, 2022

Hi @furkanmutlu it is likely frustrating to not get any feedback on this; it would be for me. However, the right person to respond would be the maintainer of the project or someone with merge permission. That would also be a good person to decide if more refactoring would result in a cleaner codebase (my last feedback) or if this current solution is preferred.

@furkanmutlu
Copy link
Contributor Author

@tordans thank you for your quick response

@tyrasd tyrasd merged commit ad62637 into openstreetmap:develop Jun 6, 2022
@tyrasd tyrasd added the bug A bug - let's fix this! label Jun 6, 2022
tyrasd added a commit that referenced this pull request Jun 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug A bug - let's fix this!
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants