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

GoDaddy Contributions #52

Closed
wants to merge 33 commits into from
Closed

GoDaddy Contributions #52

wants to merge 33 commits into from

Conversation

shelbys
Copy link
Contributor

@shelbys shelbys commented Sep 30, 2014

Corrected generateModelDefinition() to scan for model references in remote accepts, returns, errors, and nested models
Changed addRoute() to honor X-Forwarded-Host
UI/UX cleanup for SwaggerUI
Add support for toggling Model and Schema, instead of just expanding
Added guard against null mockSignature
Added support for toggling Model and Schema, and added support for primitives in StatusCodeView
Changed to hide the message-bar when no message
Changed to use defaultValue, if present, for sampleValue
Changed to use relative path for images/throbber.gif for mounting into apps at different levels
Collapse and label responseModel description by default
Correct description of collections of object and nested
Corrected to avoid showing invalid Model and Model Schema is missing or void
Corrected to load images relative to swaggerRoot, for nested context root
Corrected to work when loading resources from file://
Ensure Response Content Type is shown regardless of Response Class
Omit divs for info.title and info.description if they're absent
Removed 'Implementation Notes' label since it's just noise, widened Resource expansion anchor to full label

Tasks

shelbys and others added 30 commits July 9, 2014 03:36
* upstream/master: (38 commits)
  Bump version
  res.send deprecated - updated to res.status
  Remove hidden properties from definition.
  Bump version
  Ensure models from relations are included
  1.2.4
  model-helper: handle arrays with undefined items
  1.2.3
  model-helper: handle array types with no item type
  Bump version
  Properly convert complex return types.
  Bump version
  Fix up loopback.rest() model definition hack.
  Bump version and update deps
  s/accessToken/access_token in authorization key name
  Fix resources if the explorer is at a deep path.
  Fix debug namespace, express version.
  Remove forgotten TODO.
  Simplify `accepts` and `returns` hacks.
  More consise type tests
  ...

Conflicts:
	public/css/loopbackStyles.css
	public/css/screen.css
	public/index.html
	public/lib/loadSwaggerUI.js
	public/lib/swagger.js
	public/swagger-ui.js
	public/swagger-ui.min.js
…sponseMessages, consumes+produces, and X-Forwarded-Proto for reverse-proxying from HTTPS to HTTP
* upstream/master:
  Bump version
  Set up default consumes/produces media types
  Fix the default opts
  Add required swagger 1.2 items property for property type array
  Allow passing a custom protocol.

Conflicts:
	lib/swagger.js
Added option `swaggerDistRoot` for specific file overrides.

Conflicts:
	README.md
	example/simple.js
	index.js
	package.json
	public/css/screen.css
	public/images/logo_small.png
	public/index.html
	public/lib/loadSwaggerUI.js
…anged to always and only use responseMessages
Upgraded to v2 and ported changes from fork
<div class="contentWell">
<div id="message-bar" class="swagger-ui-wrap">&nbsp;</div>
<div id="swagger-ui-container" class="swagger-ui-wrap"></div>
</div>
Copy link
Member

Choose a reason for hiding this comment

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

@shelbys I am trying to understand the purpose of this change. I can see that you are adding left & right padding of 30px for these two elements in loopbackStyles.css.

I don't understand why are you adding a wrapping div element for that, why can't you simply apply the extra padding to those two wrapped elements directly?

#message-bar, #swagger-ui-container {
  padding-left: 30px;
  padding-right: 30px;
}

Copy link
Member

Choose a reason for hiding this comment

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

I'll go with what I proposed in my comment above - see #61

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@bajtos - I'm open to alternatives for this. The reason I added this padding at all was caused by collapsing the padding in shelbys/swagger-ui in order to support both top-level and nested-in-div usage scenarios. For instance, you can see our nested case at: https://developer.godaddy.com/doc#!/domains/domain_suggest

@@ -21,7 +21,7 @@
"url": "https://github.com/strongloop/loopback-explorer/issues"
},
"devDependencies": {
"loopback": "1.x",
"loopback": "git+https://github.com/shelbys/loopback.git",
Copy link
Member

Choose a reason for hiding this comment

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

This MUST be fixed before landing the patch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@raymondfeng - NOTE, we only forked loopback to easily reference our forks of the submodules (e.g. loopback-boot, loopback-explorer, strong-remoting, etc), so this change can be reverted before merge

@bajtos
Copy link
Member

bajtos commented Oct 15, 2014

Thank you @shelbys for answering my comments. Here is what I would like to do next: I'll continue with reimplementing the changes from this PR as I understand them. Once it's done, I will kindly ask you to test the new explorer version and let us know if my patches have covered all your needs. Is that ok with you?

@slnode
Copy link

slnode commented Oct 15, 2014

Test PASSed.

@shelbys
Copy link
Contributor Author

shelbys commented Oct 16, 2014

@bajtos - Yep, that works for me

@bajtos
Copy link
Member

bajtos commented Oct 16, 2014

@shelbys all changes from this pull request should be landed on master now. The only thing remaining to do (besides swagger-ui changes) is to support X-Forwarded-Proto header (#57), but that can be worked around via opts.protocol.

Could you please verify that I the current master works for you and that we can close this pull request as resolved?

@bajtos bajtos added the waiting label Oct 16, 2014
@bajtos
Copy link
Member

bajtos commented Oct 17, 2014

@shelbys I am closing this pull request. Please create new issues if you find anything that does not work in master the same way as in your fork.

@bajtos bajtos closed this Oct 17, 2014
@bajtos bajtos removed the #wip label Oct 17, 2014
@bajtos bajtos removed the waiting label Dec 18, 2014
@bajtos bajtos changed the title WIP: GoDaddy Contributions GoDaddy Contributions Dec 18, 2014
This pull request was closed.
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.

7 participants