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

Implement POC for image embedding #509

Open
wants to merge 3 commits into
base: master
from
Open

Conversation

@mgreter
Copy link

@mgreter mgreter commented Nov 29, 2016

Hi all

I've had the same feature wish as #364 and #137 to be able to embed some images in WorkSheets. Took me a few hours to get something up and running and I thought I'd share the results here. Unfortunately this project seems rather dead, so I did not put too much effort into this Pull Request. I might refine it a bit more once I'm actually start using it. For now I just made all changes in the main joined source file!

I also found a similar PR in another fork: protobi#19

Usage:

workbook.Sheets['my-sheet']['!images'] = [
	{
		name: 'image1.jpg',
		data: image[0].base64,
		opts: { base64: true },
		position: {
			type: 'twoCellAnchor',
			attrs: { editAs: 'oneCell' },
			from: { col: 2, row : 2 },
			to: { col: 6, row: 5 }
		}
	},
	{
		name: 'image2.jpg',
		data: image[1].base64,
		opts: { base64: true },
		position: {
			type: 'twoCellAnchor',
			attrs: { editAs: 'oneCell' },
			from: { col: 2, row : 10 },
			to: { col: 6, row: 14 }
		}
	}
];

Very helpfull references:
http://officeopenxml.com/drwOverview.php

Hope it's usefull to some people!

@stewartsims
Copy link

@stewartsims stewartsims commented Dec 13, 2016

👍 This would be very useful for a project we are involved with and as far as I can see from some research the only viable existing implementation for writing 'drawings' into spreadsheet documents in the browser. If the maintainers of this project are watching, please review these changes and merge if possible. I will test this out for our requirements.

@stewartsims
Copy link

@stewartsims stewartsims commented Dec 14, 2016

I have tested and this works fine. Took me a little while to realise I needed to chop off the 'data: image/png...' header and just supply the base64 string itself.

Of course further functionality could be added to improve - for example different options other than 'twoCellAnchor' for positional layout of the image. But this seems like a good approach to me and relatively low risk?

@mgreter
Copy link
Author

@mgreter mgreter commented Dec 14, 2016

Thanks for trying it out. I will unfortunately not use this library in the foreseeable future myself, so I don't intend to further improve this PR. As you figured out, most edges are not polished and would need further work/documentation. Sadly I don't expect the repo owner to merge this anytime, given the list of other open PRs and Issues. But cool to hear that you got it working 👍 !

If anyone wants to add more functionality to this PR, feel free to create a PR against my branch, so it will also be visible/included here.

@stewartsims
Copy link

@stewartsims stewartsims commented Dec 15, 2016

Understood - I wouldn't really expect you to add more functionality to this - but I think it will prove useful for us so thanks for submitting it to this project. As you say the seems to be stagnant for now, I'll keep an eye on it and try to get involved if I can in future.

@stewartsims
Copy link

@stewartsims stewartsims commented Dec 15, 2016

Found a couple issues with using multiple images, all easily resolvable. Will create a PR against your version shortly.

stewartsims and others added 2 commits Dec 15, 2016
fixing multiple images, keys method and adding row height config
@mgreter
Copy link
Author

@mgreter mgreter commented Dec 15, 2016

Thanks @stewartsims for the PR. I've merged it and your changes should be part of this PR now 🚀

For completeness I'll include @stewartsims comments below:

  • I noticed when using multiple images the first one was repeated: just a couple places where the index of the image was not being respected during iteration

  • Added the addition from another PR for specifying row height e.g.

ws['!rows'] = [
    {hpx:275},
    {hpx:100},
    {hpx:100}
];
  • I found the keys method threw an error in some situations when it was being passed a 'null' argument, just made it more defensive but possibly a symptom of a problem elsewhere in the code
@tarwich
Copy link

@tarwich tarwich commented Dec 17, 2016

Hey, guys. Can you add a pull request to tarwich/js-xlsx? To be honest, I haven't read this fully, but I see it's needed. I don't have a lot of time to maintain, but I'm trying to merge in these pull requests in order to keep the ball rolling for the community. Also, if anyone wants to help I'll be happy to add them.

I drafted a pull request here, so if it's what is needed, I'll go with it.
tarwich/js-xlsx@master...mgreter:master

@stewartsims
Copy link

@stewartsims stewartsims commented Dec 19, 2016

OK I've submitted a PR with details of the changes @tarwich - hope that helps.

@tarwich
Copy link

@tarwich tarwich commented Dec 20, 2016

Merged

@mgreter
Copy link
Author

@mgreter mgreter commented Dec 22, 2016

Thanks @tarwich. FWIW I thought I might add how I used this lib in my POC.
Basic request was to enter data and take pictures on mobile and store as excel.
With js-xlsx and this PR it was quite easy to do so!
It's not self contained but you should get the idea:

<!-- this is part of an angular2 template -->
<input type="file" accept="image/*" capture="camera" (change)="onPicChange(pic)" #pic />
// method is part of a class!
onPicChange(data: any) : void {
  var self = this;
  var reader  = new FileReader();
  reader.addEventListener("load", function () {

    var img = new Image();
    var canvas = document.createElement("canvas");
    var ctx = canvas.getContext("2d");

    img.onload = function()
    {
      var maxWidth = 300;
      var width = img.width;
      var height = img.height;
      if (width > maxWidth) {
        height *= maxWidth / width;
        width = maxWidth;
      }
      canvas.width = width;
      canvas.height = height;
      ctx.drawImage(img, 0, 0, img.width, img.height, 0, 0, width, height);
      self.picSrc = canvas.toDataURL("image/jpeg");
      self.picBlob = self.picSrc.split(',')[1];
    }

    img.src = reader.result;

  }, false);
  reader.readAsDataURL(data.files[0]);
}

Then it follow the example given initially:

workbook.Sheets['Order-' + id]['!images'] = [
  { name: 'image1.jpg',
    data: this.picBlob,
    opts: { base64: true },
    position: {
      type: 'twoCellAnchor',
      attrs: { editAs: 'oneCell' },
      from: { col: 2, row : 2 },
      to: { col: 6, row: 5 }
    }
   },

I also stored the image data in an offline storage in base64 (not efficient, but works).

HTH

@willemmulder
Copy link

@willemmulder willemmulder commented Jan 6, 2017

It would be great if this could be merged!

@JohnRSim
Copy link

@JohnRSim JohnRSim commented Mar 17, 2017

I agree would be great if this could be merged in.. 👍

@SheetJSDev
Copy link
Contributor

@SheetJSDev SheetJSDev commented Mar 21, 2017

Here's a quick sample to test a few features:

BIFF8 XLS

BIFF5 XLS

XLSX

XLSB

XLML

ODS

FODS

The different workbook formats use different image filetypes (is that really a surprise?), like BMP and WMF/PICT and PNG/JPG and PDF.

Immediately a few things stand out:

  1. Mandate a specific image format or allow any image format?

  2. The files are referenced at a workbook level, so we need a structure for supporting those assets. Probably have an assets hash at the workbook level, use strings in the worksheet !images to reference the assets.

  3. Which features should be supported? For example, Excel 2016 supports certain 3d effects.

@mgreter as you probably saw, it's not hard to set up the relevant fields in the XLSX to make the feature work; the challenging question is settling on a sensible representation

@oreillyj1
Copy link

@oreillyj1 oreillyj1 commented Apr 25, 2017

apologies on this but is there a small complete example of exporting html table data to XLSX (with a image in it)? somewhere out there ...a jsfiddle or something...

@FrankBirik
Copy link

@FrankBirik FrankBirik commented Apr 28, 2017

Hi,
Can I have a bit more explanation on !images? I added it but it gives me cannot set property

"The files are referenced at a workbook level, so we need a structure for supporting those assets. Probably have an assets hash at the workbook level, use strings in the worksheet !images to reference the assets."

@PranayShah
Copy link

@PranayShah PranayShah commented May 31, 2017

This works perfectly fine for me. I am using xSirrioNx's fork and had to merge in minor changes from @stewartsims commits to support images on multiple sheets. The image (a PNG) however stretches (not even scale proportionately) to the cell's dimensions. Is it possible to fix dimensions for the image?

@bys-wallance-zhang
Copy link

@bys-wallance-zhang bys-wallance-zhang commented Jul 25, 2017

@mgreter ,
Hi mgreter,I find 2 issues, one is the image only be inserted 1st sheet, if add the 2nd the image not be present in export file;
another issue is when heperlink and image add the same sheet, the image will be removed from the export file

@SheetJSDev SheetJSDev force-pushed the SheetJS:master branch to 54f380e Aug 8, 2017
@dbgtx
Copy link

@dbgtx dbgtx commented Nov 15, 2017

So I tried the @mgreter solution as listed above & his xlsx.js fork. The 3 JPEGs I tried get corrupted since I get the following error when opening the xlsx in Excel2013/16:
"We found a problem in some of the content in 'text.xlsx'. Do you want to recover as much as we can?...."

After clicking: YES, I'm notified:
Removed Part: Drawing shape. error code:

<removedParts summary="Following is a list of removed parts:">
<removedPart>Removed Part: Drawing shape.</removedPart>
</removedParts>

When looking at the xlsx zipped package before agreeing to the above, the image does appear inside the media folder, but isn't readable either. Any suggestions? What am I missing?

@SheetJSDev SheetJSDev force-pushed the SheetJS:master branch from 8508ad4 to 88e9e31 Feb 21, 2018
@SheetJSDev SheetJSDev force-pushed the SheetJS:master branch from 4cc8cff to 975c7fb Mar 19, 2018
@SheetJSDev SheetJSDev force-pushed the SheetJS:master branch from 0786b99 to 3b589f0 Aug 12, 2020
@SheetJSDev SheetJSDev force-pushed the SheetJS:master branch from 4254ed4 to eec93b0 Oct 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

You can’t perform that action at this time.