Skip to content

add comment support #529 #823

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

Merged
merged 1 commit into from
May 22, 2019
Merged

add comment support #529 #823

merged 1 commit into from
May 22, 2019

Conversation

ilimei
Copy link

@ilimei ilimei commented May 19, 2019

Fixes issue #529

@guyonroche
Copy link
Collaborator

@ilimei thank you for your contribution - this looks good. Could you do one thing? Please use 2 spaces for indents - I know it's pedantic but it helps with consistency

@ilimei
Copy link
Author

ilimei commented May 22, 2019

@guyonroche sorry for that, i will do it later ^_^

@guyonroche guyonroche merged commit fd6189c into exceljs:master May 22, 2019
@guyonroche
Copy link
Collaborator

@ilimei after some futher testing, I've started working on some changes...

  1. changing the Cell property name from "comment" to "note" -> this reflects the change in Excel from comments to "Threaded Comments". The old legacy comments are called Notes
  2. It turns out that each note (or comment) requires the addition of a vml drawing for each cell note. Sometimes the design decisions of MS Excel developers astounds me!

@guyonroche
Copy link
Collaborator

Changes are in and published to 1.12.0
@ilimei thanks for your contribution

@alexcrack
Copy link

alexcrack commented Jul 23, 2019

Hello.

  1. Comment's names in the interface file *.d.ts (and in @types/exceljs module) called 'comment' (not 'note') as before.
  2. Comments are not popupable. They are displayed constantly. But Excel (and Libre Calc) CAN do them popup on cell hover. How can I code behavior this way?

Can't you check it please?

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.

4 participants