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.

Sign up for GitHub

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

Jump to bottom

Support nested columns feature #1889

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Support nested columns feature #1889

wants to merge 7 commits into from

Conversation

jeka1985
Copy link

@jeka1985 jeka1985 commented Nov 3, 2021 •
edited
Loading

Summary

Thanks for a great lib!

Used it for generating styled dynamic reports and found generating dynamic nested headers pretty complicated because of low-level methods for adding rows and manual cell merging.

It would be great to have some declarative way for such tasks. Imho nested JSON tree looks like a solution.

In a nutshell, recursive walk the tree, collect items meta (such as cell size, including aggregated size of all children). Build flat list, use it for generating header rows and columns.

Test plan

Backward compatibility is not affected, added new worksheet method makeColumns for building flat columns from nested json tree

worksheet.makeColumns([
  {
    id: 1, 
    title: 'Some', 
  },
  {id: 2, title: 'Qwe'},
  {id: 3, title: 'Foo'},
  {
    id: 4,
    title: 'Zoo',
    child: [
      { id: 41, title: 'Zoo 1' },
      { id: 42, title: 'Zoo 2' },
      { id: 44, title: 'Zoo 3' },
      { id: 45, title: 'Zoo 4' },
    ]
  }
]);

Also you can pass column props such as width ad style to "leaf" tree items.
This input will be converted into 3 native entity groups

  • Columns
  • Rows
  • Merge rules
makeColumns(input) {
    const flatter = new ColumnFlatter(input);
    const merges = flatter.getMerges();
    this.columns = flatter.getCoumns();

    this.views.push({state: 'frozen', ySplit: flatter.getRows().length});
    
    this.addRows(flatter.getRows().map(row => row.map(item => item?.title || item?.id || null)));

    merges.forEach(item => {
      this.mergeCells(item);
    });
}

Related to source code (for typings update)

@jeka1985 jeka1985 changed the title Support nested colums feature Support nested columns feature Nov 3, 2021
@jeka1985 jeka1985 mentioned this pull request Nov 3, 2021
Open
@Siemienik Siemienik self-requested a review November 7, 2021 23:23
@Siemienik Siemienik requested review from Siemienik and removed request for Siemienik November 14, 2021 20:48
@jeka1985
Copy link
Author

@Siemienik sorry to bother you, but is there any updates on this?

@Siemienik
Copy link
Member

Siemienik commented Nov 16, 2021 •
edited
Loading

Thank you for your effort. Currently, I've no time. I'm going to see this in the first free-time slot.

Siemienik
Siemienik reviewed Nov 21, 2021
View reviewed changes
README.md Outdated Show resolved Hide resolved
@Siemienik
Copy link
Member

Wow, it is really great feature. I see that I have to analyze it deeply and the relations between nested columns and other features like borders, merged cells, styles etc. please don't blame me that It takes me a lot of time.

@jeka1985
Copy link
Author

Wow, it is really great feature. I see that I have to analyze it deeply and the relations between nested columns and other features like borders, merged cells, styles etc. please don't blame me that It takes me a lot of time.

Sure, no problem, take your time )

@jeka1985
Copy link
Author

jeka1985 commented Dec 16, 2021 •
edited
Loading

@Siemienik Just wondering about estimates, looking forward to switch to nested-columns released version and drop off ad-hoc solution :)

@jeka1985
Copy link
Author

@Siemienik ?

@AlFalahTaieb
Copy link

This is awesome @jeka1985 Thank you

@jeka1985
Copy link
Author

jeka1985 commented Feb 3, 2022

@alubbe @Alanscut Hi guys, looks like Pawel still have no time for this PR, could you please take a look on this?

@Siemienik Siemienik self-assigned this Apr 6, 2023
@vaynevayne
Copy link

How is the progress?

@Siemienik

This comment was marked as outdated.

Sign in to view
@dmk1111
Copy link

dmk1111 commented Nov 24, 2023

@Siemienik any updates on this? This fixes 6-year old issue (#365) but still not merged

Siemienik
Siemienik reviewed Nov 25, 2023
View reviewed changes
lib/doc/worksheet.js
const merges = flatter.getMerges();
const rows = flatter.getRows();

this.columns = flatter.getCoumns();
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
this.columns = flatter.getCoumns();
this.columns = flatter.getColumns();

lib/utils/column-flatter.js
*
* Filter off all cells with "children" property
*/
getCoumns() {
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
getCoumns() {
getColumns() {

@Siemienik
Copy link
Member

@Siemienik any updates on this? This fixes 6-year old issue (#365) but still not merged

I'm aware of that. Everything I plan to merge must be thoroughly tested to prevent regressions, which is time-consuming. This is more of a volunteer effort for me, and even though I do my best, there are many other tasks to prioritize.

By the way, all of you can help contribute to this project by:

  • Being active on our Discord and responding to other users' questions.

SiemaTeam

  • Reviewing PRs and adding a thumbs-up 👍 to those that are most important to you. I prioritize PRs for review based on the number of thumbs-ups they receive.
  • Being active in pinned discussions – these are the most important topics for me as a maintainer. Your active participation there helps me make decisions faster, more accurately, and wisely.

If you're interested in the project's progress, you can check out:
#2351
ExcelJS v4.4.0 Release Notes

I would also like to suggest these discussions as candidates for inclusion in the next release:
#2573
#2574

@Siemienik Siemienik mentioned this pull request Nov 25, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Siemienik Siemienik Siemienik left review comments

@huanshaolin huanshaolin huanshaolin approved these changes

At least 1 approving review is required to merge this pull request.

Assignees

@Siemienik Siemienik

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@jeka1985 @Siemienik @AlFalahTaieb @vaynevayne @dmk1111 @huanshaolin