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

Conversation

@jdestefano-mongo
Copy link
Contributor

@jdestefano-mongo jdestefano-mongo commented Feb 23, 2018

…den: true' property to hide tab strips. Added 'tabs-top' directive to pull tab strip to the top of the page.


This change is Reviewable

@jdestefano-mongo
Copy link
Contributor Author

MUST BE MERGED WITH:
mongodb/docs#3241

Copy link
Contributor

@i80and i80and left a comment

Choose a reason for hiding this comment

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

Largely lgtm! A type issue though

this.type = null;
if (this.tabStrip !== null) {
this.type = this.tabStrip.getAttribute('data-tab-preference');
if (this.tabStrips !== null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

this.tabStrips will never be null given the above switch to querySelectorAll.

This should read if (this.tabStrips.length > 0) {


setup() {
if (!this.tabStrip) { return; }
if (!this.tabStrips) { return; }
Copy link
Contributor

Choose a reason for hiding this comment

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

See above


update() {
if (!this.tabStrip) { return; }
if (!this.tabStrips) { return; }
Copy link
Contributor

Choose a reason for hiding this comment

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

See above

Copy link
Contributor

@i80and i80and left a comment

Choose a reason for hiding this comment

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

If you've tested this out, I don't see anything wrong 👍

…den: true' property to hide tab strips. Added 'tabs-top' directive to pull tab strip to the top of the page.
@jdestefano-mongo jdestefano-mongo merged commit cb54d03 into mongodb:master Feb 27, 2018
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.

2 participants