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

V1.5.0 dev #300

Merged
merged 18 commits into from
Oct 6, 2018
Merged

V1.5.0 dev #300

merged 18 commits into from
Oct 6, 2018

Conversation

jwildfire
Copy link
Contributor

This release focuses exclusively on updates to the explorer object. Biggest feature update is a new file loader accessed with explorer.config.fileLoader = true. Lots of other smallish technical updates including callbacks and easier ways to customize all codebooks created within an explorer.

@jwildfire jwildfire self-assigned this Sep 6, 2018
@jwildfire jwildfire requested a review from samussiah September 6, 2018 22:44

var files = explorer.dataFileLoad.loader.node().files;
//clear the file input
loadStatus.text('Loaded.').style('color', 'green');
Copy link
Contributor

Choose a reason for hiding this comment

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

should update this to say Loaded . instead of replacing the Choose a File text. Captured in #303 .

@@ -41,7 +41,9 @@ export function makeCodebook(explorer) {

explorer.codebook.on('complete', function() {
explorer.fileListing.init(explorer);
initFileLoad.call(explorer);
if (explorer.config.fileLoader) {
Copy link
Contributor

Choose a reason for hiding this comment

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

that's pretty darn to-the-point

f[explorer.config.labelColumn] ===
explorer.current[explorer.config.labelColumn]
)
.filter(f => f.fileID === explorer.current.fileID)
Copy link
Contributor

Choose a reason for hiding this comment

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

could also do .classed('selected', f => f.fileID === explorer.current.fileID) because i like to nitpick

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 would be cleaner. I'll commit the update.

f.settings = f.settings || {};
f.fileID = i;
Copy link
Contributor

Choose a reason for hiding this comment

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

0-indexed file IDs!?

makeCodebook() {}
};

explorer.on = function(event, callback) {
Copy link
Contributor

Choose a reason for hiding this comment

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

i. love. callbacks.

@@ -1,6 +1,7 @@
const defaultSettings = {
ignoredColumns: [],
meta: [],
defaultCodebookSettings: {},
Copy link
Contributor

Choose a reason for hiding this comment

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

effectively a single codebook settings object that applies to all codebooks? rather than individual settings?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nailed it. 🔨

Copy link
Contributor

@samussiah samussiah 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! Just one suggestion for the file loader, captured in #303 .

@samussiah samussiah requested a review from emmorris September 13, 2018 19:39
@emmorris emmorris added this to the v1.5.0 milestone Sep 14, 2018
@jwildfire
Copy link
Contributor Author

@emmorris @danedexF5 I think we can probably skip (or do a very minimal) regression testing for this release since it only applies to the explorer object which isn't in our current production workflow. I've added a column in the project to indicate which issues need feature testing.

@jwildfire jwildfire requested review from danedexF5 and removed request for danedexF5 and emmorris October 6, 2018 15:32
@jwildfire jwildfire merged commit 24fffc6 into master Oct 6, 2018
@jwildfire jwildfire deleted the v1.5.0-dev branch October 6, 2018 15:57
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.

3 participants