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

Supports setting cell comment properties #1159

Merged
merged 6 commits into from
May 23, 2020

Conversation

Alanscut
Copy link
Member

@Alanscut Alanscut commented Mar 9, 2020

changes

  • The purpose of this PR is to support the setting of cell comment properties. Currently only the protection, margin and object position properties are supported.
  • Please let me know if this PR needs anything else.

@Siemienik
Copy link
Member

Siemienik commented Mar 31, 2020

@Alanscut, many changes are here, should we make a review session through zoom.us?

@Alanscut
Copy link
Member Author

Alanscut commented Apr 7, 2020

@Siemienik Sorry for replying so late. I am looking forward to video communication with you, but English is not my native language, video communication will be very difficult for me. I would rather explain my purpose and plan in writing, hope you not mind.

Purpose:

excel.js currently only supports inserting annotations, but does not allow users to set the corresponding properties of annotations. This PR is to solve this problem.

Code:

This PR will support setting protection, margin and object position attributes. I also want to support font, alignment, size and color and line attributes, so that users can insert comments flexibly.

  • lib / utils / under-dash.js
    deepMerge () is to deeply merge the attributes of the source object to the current object without interfering with each other.

  • lib / xlsx / xform / comment / style / vml-position-xform.js
    This file is to support location object properties.

  • lib / xlsx / xform / comment / style / vml-protection-xform.js
    This is to support protection attributes.

  • lib / xlsx / xform / comment / vml-textbox-xform.js
    This is to support margin attributes.

This PR is a new feature and will not affect the original function, and all test cases are successful.

Demo

const Excel = require('../../lib/exceljs.nodejs');
const workbook = new Excel.Workbook();
const ws2 = workbook.addWorksheet('sheet1');
ws2.getCell('A1').value = 'Hello';
ws2.getCell('A1').note = 'Hello, ExcelJS!';
ws2.getCell('A5').value = 'world';
ws2.getCell('A5').note = {
  texts: [{
    'font': {
      'size': 12,
      'color': {
        'theme': 0,
      },
      'name': 'Calibri',
      'family': 2,
      'scheme': 'minor',
    },
    'text': 'hello exceljs',
  }],
  margins: {
    insetmode: 'custom',
    inset: [0.55, 0.55, 0.45, 0.45],
  },
  protection: {
    locked: 'True',
    lockText: 'True',
  },
  editAs: 'absolute',
};
const fileName1 = './read/issue_comments_padding.xlsx';
workbook.xlsx.writeFile(fileName1).then(() => {
  console.log('ok');
});

XML

// vmlDrawing1.vml
<v:shape id="_x0000_s1026" type="#_x0000_t202" style="position:absolute; margin-left:105.3pt;margin-top:10.5pt;width:97.8pt;height:59.1pt;z-index:1;visibility:hidden" fillcolor="infoBackground [80]" strokecolor="none [81]" o:insetmode="custom">
    <v:fill color2="infoBackground [80]" />
    <v:shadow color="none [81]" obscured="t" />
    <v:path o:connecttype="none" />
    <v:textbox style="mso-direction-alt:auto" inset="5.5mm,5.5mm,4.5mm,4.5mm">
      <div style="text-align:left" />
    </v:textbox>
    <x:ClientData ObjectType="Note">
      <x:MoveWithCells />
      <x:SizeWithCells />
      <x:Anchor>1, 6, 3, 14, 3, 2, 7, 16</x:Anchor>
      <x:Locked>True</x:Locked>
      <x:AutoFill>False</x:AutoFill>
      <x:LockText>True</x:LockText>
      <x:Row>4</x:Row>
      <x:Column>0</x:Column>
    </x:ClientData>
  </v:shape>

Copy link
Member

@Siemienik Siemienik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @Alanscut. I've just reviewed it deeply and have only one comment here:)

@alubbe
Copy link
Member

alubbe commented May 20, 2020

@Siemienik what do you think?

@Siemienik Siemienik merged commit a982a25 into exceljs:master May 23, 2020
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.

3 participants