JanMarvin / openxlsx2

openxlsx2 - read, write and modify xlsx files
https://janmarvin.github.io/openxlsx2/
Other
117 stars 9 forks source link

[WIP] begin hyperlink rewrite #1137

Closed JanMarvin closed 2 weeks ago

JanMarvin commented 2 weeks ago

1) do not remove hyperlinks from wb$worksheet_rels 2) do not replace wb$worksheets[[sheet]]$hyperlinks with hyperlink objects 3) save what is available

JanMarvin commented 2 weeks ago

@olivroy , I would like to know what you think about wb_add_hyperlink(). It's a newly introduced wrapper to create what ooxml calls “shared hyperlinks”. These “shared hyperlinks” are basically “builtins” for =HYPERLINK(). The hyperlink formula is what we currently use to create hyperlinks. I added wb_add_hyperlink() similar to wb_add_formula() so that the function writes the data to the cell instead of just writing an overlay.

But I'm not sure if I'm really happy with this approach, as it bundles wb_add_data_table() and wb_add_data() internally and I'm not really sure if I want that. Especially since this is related to wb_dims() and the input and output dims might be different.

On the pro side ... it works as expected, which is nicer than before, although at the moment the references are not yet shared. This would require identifying unique strings in the input and matching them.

olivroy commented 2 weeks ago

I will have to take a deeper look, but first thing, having x before dims would be better for consistency with other methods.

I am in favour here I think as I had difficulty using hyperlink. I remember having opened a PR adding an example on how to do it.

JanMarvin commented 2 weeks ago

Thanks, it’s still work in progress. The order should be cleaned up, I agree.

JanMarvin commented 2 weeks ago

I have taken another approach now. There was to much going on in the first attempt. Now it is required to write the data into the sheet and pass a valid dimension to wb_add_hyperlink(). Only thing, I'm not yet happy with, is the selection of dims with col_names = TRUE, in case someone want's to use the column name with target or tooltip.

Probably in most cases people want to create hyperlinks not for columns, but for vectors/dimensions and it would be possible to switch to col_names = FALSE and if required, people can still enable it (or use A1 notion column names).

olivroy commented 2 weeks ago

Ok, yeah, it changes a bit the internal structure of the wbWorkbook object? Maybe it requires a clear explanation for people who may have relied on other workarounds till now to create / access hyperlinks.

JanMarvin commented 2 weeks ago

As usual, I worked more on the implementation. Documentation is a secondary goal 😄

JanMarvin commented 2 weeks ago

Everything should be good to go. Added documentation and NEWS entries. I'm quite happy how it has turned out. It simplifies our code, less things to stumble over, and we can provide hyperlinks that are closer to what users expect. Also something that I can remove from my invisible to-do list. I want to have a remove and/or edit function for hyperlinks, but that can wait for another PR. This one is already way to big for my personal preferences.

olivroy commented 2 weeks ago

Looks good! Great work

JanMarvin commented 2 weeks ago

Thanks! There are still a few things to think about, but for now: 🎉