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

fix wb_set_header_footer(). closes #747 #748

Merged
merged 1 commit into from
Aug 21, 2023
Merged

fix wb_set_header_footer(). closes #747 #748

merged 1 commit into from
Aug 21, 2023

Conversation

JanMarvin
Copy link
Owner

No description provided.

@JanMarvin JanMarvin merged commit bed27b7 into main Aug 21, 2023
9 checks passed
@JanMarvin JanMarvin deleted the gh_issue_747 branch August 21, 2023 22:40
@olivroy
Copy link
Collaborator

olivroy commented Aug 22, 2023

This seems to be working with wb_set_header_footer() but still bugging in wb_add_workseet().

Maybe it would be an option to leave it out of wb_add_worksheet() in favour of wb_set_header_footer()?

@JanMarvin
Copy link
Owner Author

Could you check #751 (I copied code around, wasn't even aware that it is possible to set header/footer from add_worksheet() relict from openxlsx. A better fix would be to provide some worksheet$set_header_footer() function, but to lazy.

@JanMarvin
Copy link
Owner Author

Darn, it is not even passing our own tests ... 😞

@olivroy
Copy link
Collaborator

olivroy commented Aug 22, 2023

But it's not a feature that is very useful (even less for wb_add_worksheet()) It is just the problem of providing working examples

@JanMarvin
Copy link
Owner Author

I agree, another one to ditch in the future. There are many things that were not obvious to me when I started this little adventure. For instance the strange create_comment() and write_comment() logic. It might have made sense earlier, but right now I would simply use wb_add_comment(dims = "A1", comment = "foo"), but 2 years ago I wasn't aware (most likely everything was in a broken state :)).

Now I live with it and add it to my imaginary TODO list for the next major release after 1.0. After all fixing things like this takes time as well, testing, developing etc. and rest assured, I haven't written a xlsx comment with openxlsx or openxlsx2 in my life apart from development and probably won't do so.

@olivroy
Copy link
Collaborator

olivroy commented Aug 22, 2023

Ooh, and speaking of the devil
wb_add_comment(dims = "A1", comment = "foo") this doesn't work!

Error: comment must be class wbComment or R6

@JanMarvin
Copy link
Owner Author

Yeah sorry, that was just pseudo code. It requires a create_comment() so this should work:

wb_add_comment(dims = "A1", comment = create_comment("foo"))

And hence it should be fairly simple to hide create_comment() from the user, which raises the question, why wasn't it done in the first place? Same with threads and sparklines (oh, I just copied the idea from comment, but who came up with this stuff and why?). Why don't I "fix" or "improve" it? Add another if condition for something I do not use after all? I'd love to see a imessage/whatsapp like html bubble view for thread comments. That would be a cool feature for another helper package, even though I do not use MS365 at work and therefore won't ever have access to real life thread comments 😄

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.

2 participants