Parameters sample no REACT. #16

graysonarts · 2017-10-04T18:43:06Z

No description provided.

benlower · 2017-10-04T21:43:09Z

Samples/Parameters/parameters.js

+
+  tableau.extensions.initializeAsync().then(function () {
+    tableau.extensions.dashboardContent.dashboard.getParametersAsync().then(function (parameters) {
+      parameters.forEach(processParameter);


Can you please add something to the HTML page if there are no parameters found? Something like "No parameters found. Add some parameters to this dashboard to see this work."

benlower · 2017-10-04T21:43:36Z

README.md

 # Project Frelard

-![Image of T-Flex the T-Rex](./assets/tflx.png)
+![Image of Flex the T-Rex](./assets/flex.png)


this is good :)

benlower · 2017-10-04T21:44:49Z

Samples/Parameters/parameters.js

+  const table = $('#parameterTable');
+  const tableBody = table.children('tbody');
+
+  let onParameterChange = function (evt) {


are we standardizing in our samples on 'evt' for event? fine if so just want to make sure we're consistent.

I wouldn't say it's something we necessarily need to standardize on. I've been using fooEvent , i.e. filterEvent. No preference though.

benlower · 2017-10-04T21:45:39Z

Samples/Parameters/parameters.js

+  const tableBody = table.children('tbody');
+
+  let onParameterChange = function (evt) {
+    evt.getParameterAsync().then(function (param) {


need to handle the case where there are no parameters present

Yeah this would go for all the samples. I can go through and update them all after we get the base stuff merged in if you'd prefer @RussTheAerialist. Like his sample my tables all are just empty when the object in question is missing.

I'll add something simple.

benlower · 2017-10-04T21:47:19Z

Samples/Parameters/parameters.js

+  };
+
+  let column = function (value) {
+    const row = $('<td>');


confusing to call (a table 'cell') a row

names are hard. It should be cell :)

benlower · 2017-10-04T21:52:33Z

Samples/Parameters/parameters.js

+    return row;
+  };
+
+  let allowableValues = function (value) {


would be nice to have a comment for this function to explain what it's doing...e.g. 'this will format the text and show the allowed values that can be set for the given filter'

Yeah I would second adding more comments in general, a lot of our developers aren't always javascript savvy so I've tried to go a bit heady on the comment side of things.

Yup, I'll do a commenting pass this morning.

lbrendanl · 2017-10-05T02:40:02Z

Samples/Parameters/parameters.js

+  };
+
+  let processParameter = function (p) {
+    p.addEventListener('parameter-changed', onParameterChange);


I'd use the enum value here, tableau.TableauEventType.ParameterChanged

Ah okay, I didn't know that existed. I was basing this off of the React sample and it used the string. I'll switch to the enum.

lbrendanl · 2017-10-05T02:43:54Z

Samples/Parameters/parameters.js

@@ -1 +1,64 @@
-//TODO
+$(document).ready(function () {


The convention Samm and I have been following has been to wrap the whole extension inside an anonymous function, leaving our function definitions outside of the document ready branch. I don't know enough about JavaScript to say which way is better but we should probably do to the same thing.

Is there an example of what you mean here?

lbrendanl · 2017-10-05T02:45:52Z

Samples/Parameters/parameters.js

+  const table = $('#parameterTable');
+  const tableBody = table.children('tbody');
+
+  let onParameterChange = function (evt) {


On a related note, we are using different function declaration styles. I would lean towards function foo () { }; syntax because I think it's more approachable to non-JS developers (which I'd bundle most Tableau developers into).

Sure, I don't have a preference one way or the other for either of those two points.

lbrendanl · 2017-10-05T02:48:27Z

Samples/Parameters/parameters.js

+  };
+
+  let textColumn = function (value) {
+    const row = $('<td>');


This is much cleaner syntax than I've been using, I was using the dom API not jquery, so I'll go through and update my samples.

Also if you have time pleas give mine a looksy because I'm sure there are plenty of places I can tidy up :)

lbrendanl · 2017-10-05T02:49:49Z

This looks sharp thanks again for making. Just a few areas we should sync up on so that our sample code looks more uniform.

* Added Settings Sample * added datasource sample * cleanup * moved from innerHTML to jquery.text() * added missing gitignore and removed nod_modules * Added semistandard linter * Fixed tslint errors for settings and datasources * added 0.6.1 with parameter id change * updating to use 0.6.1 * added filtering sample * Parameters sample no REACT. (#16) * removing old images and updating Flex * Fetching is complete * Fix style issues * parity with original demo, but uglier * Address code review feedback * moving to wrapping everything into an anonymous function * Initial tutorial (#17) * Initial checkin * Initial react checkin * Progressing along * Paste in old readme * React progress * Eventing * Complete functionality * Switch to modal * Add loading screens * readme updates * Part 0 with links * Part 1 readme * Fix links * More tutorials * Part 3 readme * Full res gif * Part 4 * Part 5 * Part 6 and a complete manfiest * Clean up initial readme * Better screenshot for part 3 * Completed screenshot * Delete unused test stuff * Cleanup * Add note about the react version * first round of feedback responses * Lint errors for tutorial * bug fixes

benlower and others added 2 commits October 4, 2017 12:55

removing old images and updating Flex

a756aa7

Fetching is complete

a455a6d

graysonarts force-pushed the parameters-sample-vanilla-js branch from 84a9d9a to a455a6d Compare October 4, 2017 19:57

Russell Hay added 2 commits October 4, 2017 14:01

Fix style issues

fa38717

parity with original demo, but uglier

8b69de1

graysonarts changed the title ~~WIP: Parameters sample no REACT.~~ Parameters sample no REACT. Oct 4, 2017

benlower reviewed Oct 4, 2017

View reviewed changes

lbrendanl reviewed Oct 5, 2017

View reviewed changes

Russell Hay added 2 commits October 5, 2017 08:52

Address code review feedback

2d39c51

moving to wrapping everything into an anonymous function

756bca1

lbrendanl merged commit ec67975 into tableau:V1Samples Oct 5, 2017

		@@ -1 +1,64 @@
		//TODO
		$(document).ready(function () {

Parameters sample no REACT. #16

Parameters sample no REACT. #16

Uh oh!

Conversation

graysonarts commented Oct 4, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lbrendanl commented Oct 5, 2017

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants