Do you think it is easy to implement automatic figure numeration to the plugin?

[markomanninen]

Now caption has "Figure: ", but I would rather have "Figure n: " where n should be numbered automatic according to the project image count. This would be great. Similarly it would be nice to have all figures/illustrations listed at the end of the document. If not suitable for your project, could you guide me to extend your plugin on my own fork?

[todvora]

Hi @markomanninen, I'd love to have those numbers in captions. I tried to implement it, but found out that the generation of html pages from chapters (hook page) is not called in order. So I had no chance of knowing what numbers should images get. Maybe you can find another way how to detect correct image number?

If you want to generate list of all figures, you can probably hook to the finish hook.

Anyway pull-requests are welcomed. If you want to experiment with this plugin, just clone it, make a symlink from the gitbook node_modules lib and configure the plugin in book.json. Then you can hack on the code in index.js and immediately see results.

Is there anything else I can help with?

Best regards, Tomas

[markomanninen]

Great! I will take a look to the local development kit. So far I have used only online GitBook tool.

Perhaps first I need to get the whole book as a single html page. Then I could retrieve images in appropriate order. Maybe collecting unique image elements on a separate list and compare them to the image element on your plugin to retrieve the number?

Also some extra configurations should be added to use this feature like: numbering per chapter (1.1, 1.2,... 2.1, 2.2) | numbering only (I'd like to use captions on a list at the end of the book, but no captions under the image) | ?

Thaks a lot for a constructive comment.

[markomanninen]

There is a pretty big set of changes and additions to your plugin. Should they be different plugin, or be a good fit for your one?

gitbook-plugin-image-captions by mm

Changes are:

• added support for book wide and page wide image numbering
• added support for image specific caption and attribute configuration
• added support for image list construction by book variables
• new template keywords: _PAGE_LEVEL_, _PAGE_IMAGE_NUMBER_, _BOOK_IMAGE_NUMBER_ in addition to _CAPTION_

[todvora]

Hi Marko, This is really cool! I think that your changes are perfect fit for this plugin. Just let me review your code and test it. I'll gladly merge your PR afterwards.

Thank you!

[markomanninen]

Can you see and test it before pull request or should I PR first? On the README file there are few new sections clarifying new features...

[todvora]

No, you don't have to create a PR first. I cloned your fork and tried it.

To be able to test your changes I had to hack together simple integration-tests fw for gitbook. I had no problems to test those new vars _PAGELEVEL, _PAGE_IMAGENUMBER, _BOOK_IMAGENUMBER in captions. The image-list works also ok.

I'm little bit clueless regarding those 'indexed' configuration fields. It seems very fragile. Every new chapter or image may change indexing and the plugin will add attributes or captions to wrong images. Do you have any use-cases for it?

[todvora]

I just created new branch, merged your changes and my tests. It should be easier to collaborate on the code. Some tests are failing due to added id attribute to figures. Maybe this attribute could be added only when the global registry is builded. It would keep the default output backwards compatible with old version of the plugin. What do you think?

[markomanninen]

About 'indexed' configuration fields: that is true. You need to "reindex" them if you add images before the last one. But reindexing is required per page only. Using book wide image number would be even worse. It would require indexing every time something is added before the last image of the book. Other options to give similar image specific configuration options would be:

1) use src of the image as an index/key 2) create unique key for each image manually

Src can be very complicated if used external image resources. Manual work on the other hand is, well manual :) Using page_level.page_image_number as a key is a kind of solution between these both extreme options.

Basicly I just want to configure image (and caption) properties by individual images. Some images are illustrations, some are figures, some are pictures and some are images. So caption itself may change. Some images are good to be smaller, some better to be aligned left. Markdown used on gitbook doesn't support ![image](source){attributes} syntax.

This could be also done via pure javascript file, but it would be nice to have all related content on same place, same plugin. I will give you my ebook example soon, where I have used this plugin.

What do you mean by doing figure id "when the global registry is built"? If outcome is same, meaning that the figure gets id attribute, then it sounds good. I was first creating id on separate anchor tag inside figure element. But to my eyes it works visually better if id is on a figure tag. Then image top edge doesn't get stuck on browser window top. There will be a small padding when figure id is used.

I have a small refactoring idea for the code also. This could solve the problem with injecting page_levelto the section object. Its a little bit unsecure way to use section injections. If gitbook maintainer will add new properties to the section, then page_level and href properties might get overwritten...

[todvora]

Hi Marko, Thanks for your comprehensive response. To keep you informed - I am still working on this. I had to rewrite the whole testing, adapt current tests and add new for your features.

Today I discovered a bug in initialization of images array. You are calling that.parsePage(file).then which makes the init call async. So in some cases the page hook is called before init finishes. The fix is already done, I just need to test it more.

Have you already started some work on the refactoring? I am digging in the code you are talking about. Should I clean it a little bit?

Thanks, T.

[markomanninen]

Yes, I made a commit to my repository: https://github.com/markomanninen/gitbook-plugin-image-captions/commit/9f325a0f5346990738c6cd3897dd9d464fcc769d

Refactoring didn't work as I expected, maybe a little bit cleaner code, but far from satisfied. I also noticed async problem, even I thought that calling then() -> finally() should resolve it. But with very long image lists numbering got mixed. I'm interested to see your solution to get rid of async problem.

I also made "totally" different approach with gitbook-plugin-regexplace to get similar effect, but even there I countered similar dilemma. IMHO hooks could be better on the core. But they are what they are, and we need to work with them I guess :)

[todvora]

Ok, so my changes are all integrated in branch https://github.com/todvora/gitbook-plugin-image-captions/tree/markomanninen-master. The async problem should be solved by aggregation of all promises (see this line). There should be test cases for every your feature. All of them currently passing. I've left those tests running in a cycle for about 5 hours and everything works as expected. I beleave that the async problem should be covered.

I also changed little bit the handling of page/section attributes and removed the 'problematic' injection to section. Is it what you intended to do?

The current state of the branch seems ok to me. Please check, if everything fits for you. If yes, I'll merge it and publish new release of the plugin.

Thank you!

[markomanninen]

Is it possible to get rid of this injection: section.page_level = page.progress.current.level; and pass page argument to imageWrapper function? index.js:169 affecting to index.js:41 and index.js:59.

Then it would be quite much what I was doing. Other way around is to create one more wrapper around imageWrapper and pass page parameter there and use page parameter "locally". See function processCaptions(page, that) on https://github.com/markomanninen/gitbook-plugin-image-captions/blob/master/index.js.

Nice collaboration project!

[todvora]

I missed that, thanks! Should be fixed by now. A little bit binding magic helped (see this line).

[markomanninen]

Looks promising and very Qool! I will test with very long image lists and let you know.

[markomanninen]

I think keys.sort is not needed anymore. It will actually mess order because indexes are strings with different lenghts, not numbers:index.js:156 keys.sort();

If you keep sort and try with these 5 files, you can see the effect. Compare it with commenting //keys.sort()

book.json Uploaded using ZenHub.io pictures2.md Uploaded using ZenHub.io SUMMARY.md Uploaded using ZenHub.io pictures1.md Uploaded using ZenHub.io pictures.md Uploaded using ZenHub.io

[markomanninen]

My fix to this would shorten code even more:

index.js Uploaded using ZenHub.io

[markomanninen]

Based on above comments I made one more code review. Now extra image storage sorting is not required anymore. Instead navigation had to be sorted to get files in order:

https://github.com/markomanninen/gitbook-plugin-image-captions/blob/master/index.js

[todvora]

Thanks for those improvements. Unfortunately some tests are now failing after those changes (during build, before asserts):

 gitbook-plugin-image-captions
    ✓ should not change content without images (2187ms)
    1) should create caption from alt attribute
    ✓ should read caption format from option (1871ms)
    ✓ should align caption to the left (1848ms)
    2) should prefer title attribute if available
    ✓ should ignore images with empty alt (1867ms)
    3) should ignore images with empty title and fallback to alt
    ✓ should ignore inline images (pre) (1846ms)
    ✓ should ignore inline images (post) (1858ms)
    ✓ should ignore inline images (1849ms)
    ✓ should ignore multiple images in paragraph (1832ms)
    ✓ should handle page numbers (1843ms)
    ✓ should render registry of figures (1873ms)
    ✓ should render image global index (1861ms)
    ✓ should use image specific caption (1871ms)
    ✓ should use different caption for figure and for list (1870ms)
    ✓ pass default and specific image attributes (1865ms)

[markomanninen]

Could it be because of figure id numbering fig0.1? I had changed:

images[key].nro = that.config.book.options.variables[that.options.pluginsConfig['image-captions'].variable_name].length; -> images[key].nro = that.config.book.options.variables[that.options.pluginsConfig['image-captions'].variable_name].length + 1;

This is really a matter of convention, should the first page be 0 or 1, should the first image be 0 or 1? I have thought img.index is the real index from 0 to n, but img.nro starting from 1 as well as page_level being what is provided by book page object. What do you think?

One more thing I was thinking: id="fig1.1.1" attribute is not according to css standards. figure#fig1.1 doesn't work on css file, but on the other hand figure[id="fig1.1"] works...

[todvora]

Strange, it works now. Sorry, my fault, maybe I checked one of the previous versions. The numbering seems the same. img.nro starting from 1 seems good to me. I like the changes and simplification.

You are right about the fig0.1. Maybe fig1-1 would be better. In my book I have the chapters numbered in TOC, starting also from 1 (based on CSS numbering from 1).

[todvora]

OK, I just integrated all your changes to the branch. Tests are passing, everything looks good.

What about a merge and then release?

[markomanninen]

About css standards. I added two more configuration options to alter id creation. Now you can set any prefix you want and replace dots with any character. I didn't change default concatenation, but you may want to rethink it one more time. Is it better to have fig1-1-1 format right from beginning, or let it be fig1.1.1

https://github.com/markomanninen/gitbook-plugin-image-captions/commit/d109b0217dcfa1c4983e4d35133a315b3e81a9af

BTW. can you change TOC numbering to the online gitbook? I wonder if there is a hook to determine, if generation is done for pdf format, so that I could do some extra stuff for pdf, like changing page footer and header...

[todvora]

Hi Marko, In this case I would prefer not to provide any other configuration options. It could be confusing for users.

We should choose one format, preferably valid and css supported out of the box and document it in README. One can style selected figures in css, if required.

You can provide different CSS styles for html, e-book and PDF. There you can adapt numbering. See styling section of the documentation. You can also adapt PDF footer and header. See this issue, where some of the options are listed.

[markomanninen]

That is reasonable. After all dot syntax in id selector is not that bad: http://stackoverflow.com/questions/12310090/css-selector-with-period-in-id

Figure can be referenced on css by: figure[id="fig1.1"] {} and #fig1\.1 {}

Element can be found from js: document.getElementById('fig1.1') and jquery: $('[id="fig1.1"]') or $('#fig1\\.1').

I think merge and release is all about :+1:

Oh, and thanks about styling advices, they worked well!

[todvora]

:ship: released as 0.3.0. Thank you very much for all the hard work and lots of new features. It was fun to work with you on this!

[markomanninen]

You are welcome, it was all my pleasure! Happy to see a new release on board.