Skip to content
This repository has been archived by the owner on Jul 25, 2021. It is now read-only.

For excel exports, if cell has a type class (e.g. 'te-number'), set the cell type in the export #37

Merged
merged 9 commits into from
May 2, 2017

Conversation

danielgregorywilson
Copy link
Contributor

I noticed that for excel exports, the data type was not being respected (everything formatted as a string, instead of a number/boolean). Rather than attempt to parse the JSON data with a regex, I opted to accept an optional class override of each cell ('te-string', 'te-number', 'te-boolean'), which then ensures that the data is of the proper type when viewed in Excel/Numbers. When one of these classes is placed on a , the data should be formatted correctly.

Daniel Wilson added 2 commits December 20, 2016 12:08
* feature/set-cell-types:
  For excel exports, if cell has a type class (e.g. 'te-number'), set the cell type in the export
Daniel Wilson added 7 commits January 2, 2017 18:12
…te a button for that type; similar to formats, add a tableExport 'targets' setting (e.g. "targets: {'xlsx': '#exportButton',}"), whose values are jQuery selectors of buttons to which we attach the data and event to trigger file download
* feature/target-button:
  Allow export data to be attached to a custom button, rather than create a button for that type; similar to formats, add a tableExport 'targets' setting (e.g. "targets: {'xlsx': '#exportButton',}"), whose values are jQuery selectors of buttons to which we attach the data and event to trigger file download
* feature/set-cell-types:
  For excel exports, if cell has a type class (e.g. 'te-number'), set the cell type in the export
…res that we get the correct object in the event that the button is updated by new table data between downloads
* feature/target-button:
  Parse the file blob attributes rather than calling .data(); this ensures that we get the correct object in the event that the button is updated by new table data between downloads
  Unescape html when adding to an existing button
  Only set targets if they are defined
@clarketm
Copy link
Owner

@danielgregorywilson – Sorry, I have been on a programming hiatus for the past month. I appreciate your contributions and agree that cell-type override functionality would be a resounding enhancement to the current state of the plugin. I am testing the changes now and will slate this feature for the next minor release (v3.4.0)

Copy link
Owner

@clarketm clarketm left a comment

Choose a reason for hiding this comment

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

Hey @danielgregorywilson, sorry for the 4 month hiatus. I had to take some time to address some immediate concerns on my closed source projects. I don't want you to think that I don't appreciate your contributions nor want this feature to be integrated into TableExport. To be honest, I would like to get these changes merged in sooner than later (this week if possible).

The blockage and hesitation lie in the fact that I am trying to deprecate v3.x.x in favor of v4.x.x (I'm adding a notice to the README to avoid any more unwarranted inconveniences in the future), since v4 is a complete superset of v3 and will completely supersede it in the next release and ever other release going forward. If you have the bandwidth and don't mine retargeting your PRs to v4.0.0-alpha.4, I can likely publish out your changes within a day or at the very best within a few hours after I can run tests.

I know this is short notice, but we really need this on the v4 channel since the next wave of updates (some even related to this type issue) will also go through that channel. I'd understand if you can't get these changes done before the next release cycle. Of course, I will still attribute the contributions to you, I'll just need to circumvent/rebase this specific PR, since it's pointing to the v3 channel and not v4. Let me know if this is feasible. At the very least, you could repoint the PR and I could refactor the commits; at least then, the contribution flow would be accurate - because after all this enhancement was 100% thanks to you! Thanks again for your help.

@clarketm clarketm merged commit eba4197 into clarketm:master May 2, 2017
@danielgregorywilson
Copy link
Contributor Author

Sorry for not getting back to you earlier. It looks like you have everything merged, so I don't need to take further action.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants