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: Row/CellEditor should disable inputs to avoid post parameters #3571

Closed
tandraschko opened this issue Apr 10, 2018 · 20 comments · Fixed by #11160
Closed

DataTable: Row/CellEditor should disable inputs to avoid post parameters #3571

tandraschko opened this issue Apr 10, 2018 · 20 comments · Fixed by #11160
Assignees
Labels
enhancement Additional functionality to current component ⚡ performance Performance related issue or enhancement
Milestone

Comments

@tandraschko
Copy link
Member

tandraschko commented Apr 10, 2018

see #3570
see #3564
see #3562
see #2008

@ssibitz
Copy link

ssibitz commented Apr 10, 2018

Disabling the fields by row/celleditor is also a way to prevent the datas from posting ...

@kukel
Copy link
Contributor

kukel commented Apr 10, 2018

#3564 is a separate issue. Not related to Row/Cell editing. I posted a 'fix' there (sorry, no time, or rather currently no environment to make a PR)

@kukel
Copy link
Contributor

kukel commented Apr 10, 2018

Disabling the fields makes it harder for aria stuff I think... and when you disable it for real and not for aria that might become a problem. I'm not in favour of solving it that way. Making the row/cell editor do a real partial submit is the better way. I'm already looking into these

@tandraschko
Copy link
Member Author

I'm a noob in ARIA ;) but it feels the right way as the inputs are not enabled per default actually, only when you enter the edit mode.

@kukel
Copy link
Contributor

kukel commented Apr 10, 2018

Technically they are enabled, just not visible which is for the visibly impaired not relevant. If you disable them technically but (most likely) need them not disabled for the visibly impaired, you get too much conditional stuff which makes an (at least certainly in your eyes) complex component even more complex (or is it even worse and you need to do it in all components? Would not want to go that way. Let's see if a partialSubmit on the events might help. I saw you committed a fix but reverted it?

@tandraschko
Copy link
Member Author

Yes, i didn't like that fix.

I would evaluate if dynamically disable/enable all :input elements via jquery could work.

@kukel
Copy link
Contributor

kukel commented Apr 10, 2018

I think you can use a jquery selector to find all inputs of a certain row and add that as a value to the process attribute. This leaves all other required fields in there. So the idea of your fix was not that weird... (imo)... might need a little tweaking...

@ssibitz
Copy link

ssibitz commented Apr 11, 2018

What's about something like this:

1.) In general render all datatable input-fields with the readonly tag - e.g.

disabled="disabled"
aria-disabled="true"

<div class="ui-cell-editor-input"><input id="form:cars1:0:j_idt125" name="form:cars1:0:j_idt125" type="text" value="1974" style="width:100%" class="ui-inputfield ui-inputtext ui-widget ui-state-default ui-corner-all" data-p-label="Year" data-p-con="javax.faces.Integer" role="textbox" aria-disabled="false" aria-readonly="false" disabled="disabled" aria-disabled="true"></div>

2.) When you enter the edit-mode (row or cell) you remove the disabled-tag
of the current ui-cell-editor-input :

$("[name='form:cars1:0:j_idt125']").prop('disabled', false);

$("[name='form:cars1:0:j_idt125']").prop('aria-disabled', false);

3.) When the AJAX-Post has been finished you should re-enable the disabled-tag:

$("[name='form:cars1:0:j_idt125']").prop('disabled', true);

$("[name='form:cars1:0:j_idt125']").prop('aria-disabled', true);

@ssibitz
Copy link

ssibitz commented Apr 11, 2018

Disabling all input-fields by default can also be done by
jquery in the datatable.js

Only add this to the bindEditEvents-Function works
as simple as it is for each table children :

$(".ui-cell-editor-input").children().prop("disabled", "true");

    bindEditEvents: function() {
        var $this = this;
        this.cfg.cellSeparator = this.cfg.cellSeparator||' ';
        this.cfg.saveOnCellBlur = (this.cfg.saveOnCellBlur === false) ? false : true;

        // Set all fields to disabled by default:
        $(".ui-cell-editor-input").children().prop("disabled", "true");

So with only adding this also the AddRow issue #3564
is now working right because as default no datas are send to the server
because no disabled datas are send by default.

I tried it with the showcase and instead of sending 1000 url encoded parameters
from the table when i click the AddRow-button only 5 url encoded parameters
are added and the new row will be shown ...

@kukel
Copy link
Contributor

kukel commented Apr 11, 2018 via email

@ssibitz
Copy link

ssibitz commented Apr 11, 2018

Yes, it's currently a workaround,
but what's the solution when the datatable currently can't
handle more that 50(!) rows in our project with a table that only contains 12 columns ?
We need the datatable in this style without a paginator
because the user should be able to add new "settings" dynamic.

This is currently a big show-stopper for us and our software which is planned to go
online for our customers this month, so I have to make it work as fast as possible

  • even if i currently have to fork primefaces and use a own created version for handling this issue
    until it's really fixed ...

What's the withdraw - fix ?

@ssibitz
Copy link

ssibitz commented Apr 11, 2018

I think i found out another solution which is also working
but don't changes anything for the user visually or functionally:

1.) Using a data- attribute to mark all input-fields in the data-table
2.) On post filtering out all parameters which have the data- attribute set.

So default no datas of the datatable are posted as params to the server
and in edit-mode the only TODO is to remove the data- attributes from
the current edited row or col.

This currently works for AddRow and Posting,
for the different editing-modes i will adapt this to remove the data- attribute
until the special datas have been send...

@tandraschko
Copy link
Member Author

tandraschko commented Apr 11, 2018

Would only work for ajax posts.

IMO the cleanest solution is that the roweditor will really enable/disable the inputs.

@tandraschko tandraschko changed the title Row/CellEditor should disable inputs to avoid post parameters DataTable: Row/CellEditor should disable inputs to avoid post parameters Apr 11, 2018
@ssibitz
Copy link

ssibitz commented Apr 12, 2018

Yes - that's the cleanest solution, but how to handle if the user already disabled an input
as @kukel commented ?

@ssibitz
Copy link

ssibitz commented Apr 12, 2018

If only disabling all fields by default this is quite simple by using jQuery:

$(".ui-cell-editor-input").children().prop("disabled", "true");

... on the initalizing of the datatable.

@tandraschko
Copy link
Member Author

Exactly

  1. loop all inputs
  2. if not disabled, disable them add a flag data-disabled-by-roweditor or something
  3. if entering rowediting, enable all with the data-disabled-by-roweditor flag

@kukel
Copy link
Contributor

kukel commented Apr 12, 2018

Jikes... "Cleanest solution"???? No way... IMO a terrible hack. The I currently don't have the time but I'll create a clean patch tomorrow or in the weekend (or maybe even tonight) in line with the original one by Thomas but smaller...

@kukel
Copy link
Contributor

kukel commented Apr 12, 2018

Oh, and what if in the process other inputs are updated? Then you lose the status... Adding/manipulating other inputs this way is NOT clean... Sorry...

@tandraschko
Copy link
Member Author

Ok ok, but you will never fix all issues then ;) If the datatable is processed by ajax or posted by non-ajax, there is no way to fix it.
IMO thats a design flaw that all inputs are enabled by default in this case.

@arunvc
Copy link

arunvc commented Oct 27, 2020

Partial submit and update of single rows is a great feature to have
rowIndex based data processing process=rowId update=rowId
but i am not sure if its do able with data table.

@melloware melloware added the ⚡ performance Performance related issue or enhancement label Aug 24, 2023
@melloware melloware self-assigned this Dec 20, 2023
@melloware melloware added this to the 14.0.0 milestone Dec 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Additional functionality to current component ⚡ performance Performance related issue or enhancement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants