Skip to content

Alignment improvements in the Layer control #565

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

Merged
merged 7 commits into from
Nov 18, 2021

Conversation

Malvoz
Copy link
Member

@Malvoz Malvoz commented Nov 7, 2021

  • a056aa0: re-use CSS to avoid duplicating user-select styles
  • ed8687d: align the checkbox with other controls (also removes unnecessary width and the need for an !important statement)

    Before:
    input-not-aligned
    After:
    input-aligned
  • b4f5afe: remove text-align on the layer name (because it 1: doesn't appear to be needed, and 2: doesn't look good if the map is set to RTL direction but the layer name is in an LTR script)

    Before:
    rtl-with-text-align
    After:
    rtl-without-text-align
    (i.e. now looks good independently of the direction of the map and/or the script used in the layer name, tested using sample arabic text)
  • c95d4e3: removes the indentation/margin for better alignment and improves alignment between labels and radio inputs (also fixes an RTL issue)

    Before:
    controls-alignment-before
    After:
    controls-aligntment-after
  • b6f59f0: add padding to the layer name (see screenshot below)

@Malvoz Malvoz changed the title Few Layer control CSS improvements Alignment improvements in the Layer control Nov 7, 2021
@Malvoz Malvoz requested a review from prushforth November 8, 2021 02:04
@Malvoz Malvoz marked this pull request as draft November 12, 2021 15:40
@Malvoz Malvoz mentioned this pull request Nov 12, 2021
7 tasks
@Malvoz Malvoz marked this pull request as ready for review November 12, 2021 18:38
@Malvoz Malvoz requested a review from ahmadayubi November 12, 2021 18:38
Copy link
Member

@ahmadayubi ahmadayubi left a comment

Choose a reason for hiding this comment

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

I like the alignment changes as it makes it more consistent but I guess it's up to preference.

@Malvoz
Copy link
Member Author

Malvoz commented Nov 13, 2021

c95d4e3: Removes the indentation/margin for better alignment

The original indentation was intentional, but I don't remember if not indenting the opacity slider was intentional or not. I think the logical thing is to either indent all input controls or don't indent any. While I think indenting looks better when you expand all details elements, it kind of looks strange with only having the opacity slider visible:

nested-indentations

as opposed to:

no-indentation

It's not a big deal I guess, but if you prefer one thing over the other let me know. 🙂

@prushforth
Copy link
Member

I prefer the indented version. Thanks!

@Malvoz
Copy link
Member Author

Malvoz commented Nov 13, 2021

@prushforth I've indented all the settings (including opacity slider) in 15d3ed4:

indented-inputs

Feel free to merge, or let me know if there's anything else. 👍

@prushforth
Copy link
Member

Looks good!

@Malvoz
Copy link
Member Author

Malvoz commented Nov 14, 2021

While I'm at it, pushed b6f59f0 to give the layer item name some padding in the block direction (top/bottom in this writing mode):

Before:

layer-name-padding-before

After:

layer-name-padding-after

(the effect of the padding is only visible if the layer name is long enough / layer name's container is small enough that the layer name wraps and exceeds the height of the container)

@prushforth prushforth merged commit 546571a into Maps4HTML:main Nov 18, 2021
@Malvoz Malvoz deleted the layer-control-input-align-center branch November 18, 2021 03:08
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.

3 participants