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

Support externalDocs. Closes #112 #177

Merged
merged 6 commits into from
Jul 16, 2017

Conversation

brendo
Copy link
Contributor

@brendo brendo commented Jul 14, 2017

Would like to add some additional snapshot tests before merging.

Features:

  • Handles all externalDocs scenarios described in Add support for External Docs #112
  • Styling cleanup to put padding on the containers instead of each individual child
  • Prevent description from being added if it's an empty string
  • Update CHANGELOG.

@brendo brendo force-pushed the feauture-112-external-docs-support branch from b052d55 to 4d34d0b Compare July 14, 2017 03:39
@brendo brendo force-pushed the feauture-112-external-docs-support branch from 4d34d0b to 4523256 Compare July 14, 2017 06:52
@brendo brendo changed the title WIP: Support externalDocs. Closes #112 Support externalDocs. Closes #112 Jul 14, 2017
@brendo brendo requested a review from quangkhoa July 14, 2017 06:52
@@ -68,6 +76,21 @@ export default class Page extends Component {
)
}

findTagDocs (tagHandle, navigation) {
Copy link
Contributor

Choose a reason for hiding this comment

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

With this approach, the UI component is getting 'smart', as it is now understanding the relationship between services and navigation.

Another approach is to keep the UI component really simple, and handle externalDocs at the parser level.

What do you reckon @brendo ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As discussed, bit tricky, the parser is then making assumptions that the UI can't display tag information together (which we currently can't, as the tags form the navigation).

Secondly the parser is a bit hairy to make this possible right now :(

@@ -230,7 +233,7 @@ function getUIParametersForLocation (parameters, location) {
function getUIRequest (description, requestBody = null) {
const uiRequest = {}

if (description) {
if (description && description !== '') {
Copy link
Contributor

Choose a reason for hiding this comment

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

I thought an empty string would evaluate to false, as in https://developer.mozilla.org/en-US/docs/Glossary/Falsy?

@quangkhoa quangkhoa merged commit aaf5654 into temando:master Jul 16, 2017
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.

2 participants