Skip to content
This repository was archived by the owner on Jun 18, 2024. It is now read-only.

Conversation

wdo3650
Copy link
Contributor

@wdo3650 wdo3650 commented Nov 16, 2015

Adding error banner component to Fabric.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering if we can just add a class to the chevronButton container to make it visible and have it display none by default instead of relying on js setting inline styles. Something like:

.ms-ErrorBanner-expand {
display: none;

&.is-visible {
display: inline-block;
}
}

and

var _onResize = function(event) {
_clientWidth = _errorBanner.offsetWidth;
if ((_clientWidth - _bufferSize) > _initTextWidth && _initTextWidth < _textContainerMaxWidth) {
_textWidth = "auto";
_chevronButton.className = "ms-ErrorBanner-expand";
_collapse();
} else {
_textWidth = Math.min((_clientWidth - _bufferSize), _textContainerMaxWidth) + "px";
_chevronButton.className += " is-visible";
}
_clipper.style.width = _textWidth;
};

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Resolved.

@philworthington
Copy link
Contributor

Really nice work and clean code. Just had a few comments on things. Looking good though.

William Do added 2 commits November 20, 2015 14:30
…bility. Added a CSS class to toggle the chevron button's display property as opposed to using js. Added a class to toggle the display property of the component. Added the public method 'showBanner' to show the banner if it is in a hidden state. Fixed misspelling. Removed unnecessary padding. Moved font mixin to top of main component class.
@wdo3650
Copy link
Contributor Author

wdo3650 commented Nov 21, 2015

Thanks for the suggestions! I made the changes you noted as well as made some additional changes based on your feedback.

@AngelaDiaFazio
Copy link

Go home William

On Fri, Nov 20, 2015 at 7:10 PM, William Do [email protected]
wrote:

Reopened #212 #212.


Reply to this email directly or view it on GitHub
#212 (comment).

ADF

Copy link
Contributor

Choose a reason for hiding this comment

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

Remove space between class selector and nested selector here.

@philworthington
Copy link
Contributor

This is looking great, I just called out a few small spacing things that are related to our conventions. After that you can just merge this in. Nice updates!

wdo3650 added a commit that referenced this pull request Dec 1, 2015
@wdo3650 wdo3650 merged commit e7acd5c into master Dec 1, 2015
@mikewheaton mikewheaton deleted the wdo3650/error-banner branch February 9, 2016 03:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants