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

Datatable: ColumnGroup refactoring #6409

Open
3 of 4 tasks
Rapster opened this issue Oct 13, 2020 · 7 comments · May be fixed by #9872
Open
3 of 4 tasks

Datatable: ColumnGroup refactoring #6409

Rapster opened this issue Oct 13, 2020 · 7 comments · May be fixed by #9872
Assignees
Labels
breaking-change Change is not backwards compatible enhancement Additional functionality to current component
Milestone

Comments

@Rapster
Copy link
Member

Rapster commented Oct 13, 2020

Description
Columns grouping follows a very "unconventional" approach: 13 columns have to be defined when only 5 are displayed! My guess, is that no solution have been found at the time to render nested tr/th. Consequence of this, is the current implementation makes the code very hard to maintain at very low level for us as PF developers, but also for developers using column grouping. I can see that ColumnGroup has been giving a really hard time since it's been there 😅

FYI: I believe other PrimeUI librairies should do the same (if possible)

Describe the solution you would like
Could be done 100 times simpler (colspan and rowspan automatically calculated):

                
<p:dataTable id="tblSales" var="sale" value="#{dtGroupView.sales}" columnGroupLegacyEnabled="false"
                             scrollable="#{dtGroupView.frozenColumns}" frozenColumns="#{dtGroupView.nbFrozenColumns}"
                             scrollWidth="500">

    <f:facet name="header">
        <h:outputText value="Product Sales"/>
    </f:facet>

    <p:column headerText="Product" field="manufacturer" footerText="Table footer (tfoot) using column footer facet"/>

    <p:columnGroup headerText="Sale Rate">
        <p:columnGroup headerText="Sales">
            <p:column headerText="Last Year" field="lastYearSale" footerText="A"/>
            <p:column headerText="This Year" field="thisYearSale" footerText="B"/>
        </p:columnGroup>
        <p:columnGroup headerText="Profit">
            <p:column headerText="Last Year" field="lastYearProfit" footerText="C"/>
            <p:column headerText="This Year" field="thisYearProfit" footerText="D"/>
        </p:columnGroup>
    </p:columnGroup>

    <f:facet name="tfooter">
        <p:row >
            <p:column>Table footer (tfoot) using table tfooter</p:column>
            <p:column colspan="2" style="text-align:right">Totals:</p:column>
            <p:column>
                <h:outputText value="#{dtGroupView.lastYearTotal}">
                    <f:convertNumber type="currency" currencySymbol="$"/>
                </h:outputText>
            </p:column>
            <p:column>
                <h:outputText value="#{dtGroupView.thisYearTotal}">
                    <f:convertNumber type="currency" currencySymbol="$"/>
                </h:outputText>
            </p:column>
        </p:row>
    </f:facet>

    <f:facet name="footer">
        <h:outputText value="Data between 2013-2014"/>
    </f:facet>
</p:dataTable>

Additional context
Besides it's easier to write and read (less verbose), doing so, No trick is required to make a few features like filtering and sorting work. WDYT?

Done

  • Support of frozen columns
  • Introducing new facet tfooter (tfoot html tag) allowing to define your own tfoot instead of using ColumnGroup#type

Todo

To check

@Rapster Rapster added enhancement Additional functionality to current component question labels Oct 13, 2020
@melloware
Copy link
Member

I have never used column groups so i will let @mertsincan chime in on this one.

@Rapster
Copy link
Member Author

Rapster commented Dec 27, 2020

Closing it for now, it seems with recent changes, community is fine with it 😢

@Rapster Rapster closed this as completed Dec 27, 2020
@tandraschko
Copy link
Member

I just fixed bugs for now as RC1 is near
So im fine to talk about refactorings here after the release

@Rapster
Copy link
Member Author

Rapster commented Dec 27, 2020

ok

@tandraschko tandraschko reopened this Feb 1, 2021
@tandraschko
Copy link
Member

it would be really nice to get rid of ui:repeat support but i think this is a heavy refactoring on how columnGroups works with filters.
This would also make it possible to remove forEachColumn and use FilterMeta#getColumn again

@tandraschko
Copy link
Member

@Rapster would be great if you can think about it for v11, your are refactorings in DT made it maintainable again!

@Rapster
Copy link
Member Author

Rapster commented Feb 2, 2021

it would be really nice to get rid of ui:repeat support but i think this is a heavy refactoring on how columnGroups works with filters.

On the contrary, that's why I didn't want to support it in a first place...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change Change is not backwards compatible enhancement Additional functionality to current component
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants