Skip to content

Conversation

@lbrendanl
Copy link
Contributor

This contains the necessary changes for the 0.9.0 release. This includes:

  • Updating docs based on new contract (feel free to ignore changes under docs/, this is all autogenerated).
  • Updated old samples to use new 0.9.0 library.
  • Added a new sample that show cases the UI namespace.

@lbrendanl lbrendanl requested a review from sdesmond46 February 12, 2018 22:49
sdesmond46
sdesmond46 previously approved these changes Feb 12, 2018
Copy link
Contributor

@sdesmond46 sdesmond46 left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thanks for adding all the comments to the sample

<script src="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.7/js/bootstrap.min.js" ></script>

<!-- Extensions Library (this will be hosted on a CDN eventually) -->
<script src="../../lib/tableau-extensions-0.9.0.js"></script>
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we create some sort of tableau-extensions-0.latest.js version of the bootstrap so we don't always need to modify all of these?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah that's a good suggestion, I'll do that.

});

function configure() {
const popupUrl = 'http://localhost:8765/Samples/UINamespace/uiNamespaceDialog.html';
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this use window.location to dynamically get this url?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, will add.

sdesmond46
sdesmond46 previously approved these changes Feb 14, 2018
Copy link
Contributor

@sdesmond46 sdesmond46 left a comment

Choose a reason for hiding this comment

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

Looks great!

sdesmond46
sdesmond46 previously approved these changes Feb 14, 2018
@lbrendanl lbrendanl merged commit c29c067 into master Feb 14, 2018
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.

5 participants