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

Inconsistency in variable names: topbar vs top-bar #9451

Closed
eksperimental opened this issue Dec 5, 2016 · 5 comments
Closed

Inconsistency in variable names: topbar vs top-bar #9451

eksperimental opened this issue Dec 5, 2016 · 5 comments
Labels

Comments

@eksperimental
Copy link

Hi Everyone,
first issue report here,
I just found inconsistent that you use "topbar" and "top-bar" to refer to the Top Bar component.

Pretty much it is a matter of renaming your setting variables.

scss/components/_top-bar.scss
11:$topbar-padding: 0.5rem !default;
15:$topbar-background: $light-gray !default;
17:/// Background color submenus within the top bar. Usefull if $topbar-background is transparent.
19:$topbar-submenu-background: $topbar-background !default;
23:$topbar-title-spacing: 0.45rem 1rem 0 1rem !default;
27:$topbar-input-width: 200px !default;
31:$topbar-unstack-breakpoint: medium !default;
45:  padding: $topbar-padding;
49:    background-color: $topbar-background;
52:  // Check if $topbar-background is differnt from $topbar-background-submenu
53:  @if ($topbar-background != $topbar-submenu-background) {
55:      background-color: $topbar-submenu-background;
61:    max-width: $topbar-input-width;
126:    @include breakpoint($topbar-unstack-breakpoint) {
146:      margin: $topbar-title-spacing;
158:      margin: $topbar-title-spacing;

scss/settings/_settings.scss
592:$topbar-padding: 0.5rem;
593:$topbar-background: $light-gray;
594:$topbar-submenu-background: $topbar-background;
595:$topbar-title-spacing: 0.45rem 1rem 0 1rem !default;
596:$topbar-input-width: 200px;
597:$topbar-unstack-breakpoint: medium;

I can contribute with a PR, i just need to be guided how to want to deprecate this and if you want to warn users when old-named variables are used

@kball
Copy link
Contributor

kball commented Dec 5, 2016

@ncoden @andycochran @brettsmason what do y'all think?

@andycochran
Copy link
Contributor

andycochran commented Dec 5, 2016

I'm not sure this is a bug. But this is also the case for accordionmenu and accordion-menu, buttongroup and button-group, closebutton and close-button, dropdownmenu and dropdown-menu, helptext and help-text, mediaobject and media-object, offcanvas and off-canvas … where we use one word for SASS variables and two hyphenated words for CSS classes.

@ncoden
Copy link
Contributor

ncoden commented Dec 6, 2016

If we have to be consistent now, I think we should move to lisp-case, because it is the CSS standard. However, the namespace and role of the class/variable/function/mixin can't be easily separated. This is related to the naming problem mentioned in Yetinauts/#18

@DanielRuf
Copy link
Contributor

@ncoden what is our plan, how do we handle this?

@DanielRuf
Copy link
Contributor

Closing as we will not change this in v6.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants