Skip to content

Conversation

@Warkot
Copy link
Contributor

@Warkot Warkot commented Aug 23, 2016

@Warkot Warkot changed the title XW-1687 | static global nav XW-1687 | static logged in global nav Aug 23, 2016
</svg>
</a>

<a href="#" class="wds-button wds-is-squished wds-global-navigation__start-a-wiki wds-is-secondary">Start a wiki</a>
Copy link

Choose a reason for hiding this comment

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

I feel these classes should be sorted somehow differently. Maybe like this?

wds-button wds-is-squished wds-is-secondary wds-global-navigation__start-a-wiki

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably it should be discussed here #57, where the button are added.

@rogatty
Copy link

rogatty commented Aug 23, 2016

One minor thing but LGTM

</svg>
</a>

<a href="#" class="wds-global-navigation__content-bar wds-has-fixed-width wds-is-green">Games</a>
Copy link

@rogatty rogatty Aug 24, 2016

Choose a reason for hiding this comment

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

I think we should use wds-is-games instead of wds-is-green. API will control this class and I don't think it should control the colors.

Copy link

Choose a reason for hiding this comment

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

Also, this is what we use in the footer:

<li class="wds-global-footer__links-list-item">
  <a href="#" class="wds-global-footer__link wds-is-games">
    <div>Games</div>

@kvas-damian kvas-damian changed the base branch from XW-1618 to XW-1618v2 August 24, 2016 15:05
</div>
<div class="wds-dropdown-content wds-global-navigation__dropdown">
<ul class="wds-list wds-has-lines-between">
<li><button class="wds-button wds-is-full-width">Sign in</button></li>
Copy link
Contributor

Choose a reason for hiding this comment

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

Please fix indentation

@hakubo
Copy link
Contributor

hakubo commented Aug 26, 2016

lgtm

@hakubo hakubo merged commit 56cb4e9 into XW-1618v2 Aug 26, 2016
@hakubo hakubo deleted the XW-1678 branch August 26, 2016 13:51
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.

4 participants