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

Datagrid: Adding additional span element causes incorrect editor values #4637

Closed
bempedragosa opened this issue Nov 23, 2020 · 14 comments
Closed
Assignees
Labels
priority: high team: landmark For Landmark issues type: bug 🐛 [3] Velocity rating (Fibonacci)

Comments

@bempedragosa
Copy link

Describe the bug
Inserted additional element with the info/alert text that causes cellNode.text() in sohoxi.js > makeCellEditable function returned the alert tooltip with the value.

I can also replicate it on Tag component.

To Reproduce
Steps to reproduce the behavior:

  1. Go to https://codepen.io/bemuuchan/pen/jOrRYvy
  2. Click on the editable field

Expected behavior
It will returned the original value

Actual behavior
It returned the field value with text content

Version
-ids-enterprise: [e.g. v4.34]

Platform

  • Chrome
@tmcconechy
Copy link
Member

tmcconechy commented Nov 23, 2020

What do you mean by causes cellNode.text() in sohoxi.js > ? I think usually tags and badges are non editable embellishments. Can you explain your use case a bit?

  1. Whats the point here of adding text: 1234` ?
  2. I do see it shows error so i think basically the issue is if you use the ranges it cant figure out the editor value.

@tmcconechy tmcconechy added [3] Velocity rating (Fibonacci) type: bug 🐛 labels Nov 23, 2020
@tmcconechy tmcconechy changed the title Adding additional span cause editable field to return its content with the value Datagrid: Using tags and Badges causes incorrect editor values Nov 23, 2020
@bempedragosa
Copy link
Author

bempedragosa commented Nov 23, 2020

I encountered it when I added a "< span class=audible >" element with tooltip text content on a cell with alerts. It returned the tooltip text with value. I just use the Tag component as an example.

Please also check this video for reference > https://jira.lawson.com/secure/attachment/1397574/CU75-DetailTrackedErrorMessage.mp4

For your question 1, it is the text returned for invalid range that is inserted on the "< span class=audible >" as text content.

@tmcconechy
Copy link
Member

So you just want the editor to let you edit the value you see i the cell?

@bempedragosa
Copy link
Author

Yes. Thank you!

@bempedragosa bempedragosa changed the title Datagrid: Using tags and Badges causes incorrect editor values Datagrid: Adding additional span element causes incorrect editor values Nov 25, 2020
@tmcconechy tmcconechy added the team: landmark For Landmark issues label Dec 9, 2020
@tmcconechy tmcconechy added this to To do in Enterprise 4.37.x (Jan 2021) Sprint via automation Dec 11, 2020
@tmcconechy
Copy link
Member

Hi @bemariepedragosa i noticed the example isnt working anymore to illustrate the issue. It looks broken now?

@tmcconechy tmcconechy added this to To do in Enterprise 4.37.x (Jan 2021) Sprint via automation Dec 22, 2020
@bempedragosa
Copy link
Author

Hi @tmcconechy ,
You mean this example > https://codepen.io/bemuuchan/pen/jOrRYvy ? It is still working on my end

@tmcconechy
Copy link
Member

Weird ok thanks yes it's working now , sorry about that might have been an issue on my end

@tmcconechy tmcconechy moved this from To do to In progress in Enterprise 4.37.x (Jan 2021) Sprint Jan 25, 2021
@tmcconechy tmcconechy self-assigned this Jan 25, 2021
@tmcconechy tmcconechy moved this from In progress to Pending Review in Enterprise 4.37.x (Jan 2021) Sprint Jan 25, 2021
@tmcconechy tmcconechy moved this from Pending Review to Ready for QA (beta) in Enterprise 4.37.x (Jan 2021) Sprint Jan 25, 2021
@tmcconechy
Copy link
Member

Add this on 41d0048

@CindyMercadoReyes
Copy link
Collaborator

This issue is now resolved.

@CindyMercadoReyes CindyMercadoReyes moved this from Ready for QA (beta) to Done in Enterprise 4.37.x (Jan 2021) Sprint Jan 26, 2021
@ceszilla
Copy link

hi Tim. this is one of the identified High Priority VPAT items. thanks.

@bempedragosa
Copy link
Author

Hi @tmcconechy ,
I can still replicate it on 4.3.7 . Please refer to this video > https://jira.lawson.com/secure/attachment/1397574/CU75-DetailTrackedErrorMessage.mp4

The method formatDisplayAlert within lm-formatter.service.ts inserted an additional element, that causes cellNode.text() (within makeCellEditable) to return the alert tooltip with the value.

<span class="audible">${alertTooltip}</span>

The value resets to 0 after clicking outside the field.

@tmcconechy
Copy link
Member

@bemariepedragosa it seems to be working on https://master-enterprise.demo.design.infor.com/components/datagrid/test-editable-alerts as far as i can tell. Does it work here ok for you? If so maybe you need to adjust something so whats in the video works.

Or alternatively can you tell me how to reproduce it in datagrid/test-editable-alerts ?

BTW i think this is a bit of a wierd use case and i dont think it should be done but added support for it anyways so something to think about....

@bempedragosa
Copy link
Author

Hi Tim,
I still cannot reproduce this on datagrid/test-editable-alerts because it uses the Alert formattter which also met the condition on your change. In LMCLIENT code, we're not using the Alert Formatter for cell with alerts because ranges is not always defined so it doesn't meet the Alert formatter code definition on soho.

Below is the value for formatter used in datagrid in landmark apps:
"(row, cell, value, columnDef, dataView, dataGrid) => Object(list_formatters_lm_field_formatter__WEBPACK_IMPORTED_MODULE_20_["lmFieldFormatter"])(this.lmFormatterService, this.lmFormatUtilService, row, cell, value, columnDef, dataView, dataGrid)"

Can we add additional condition for this? Like, enable this.editor.useValue if it has a class attribute datagrid-alert-icon on cellNode.

Proposed solution:
makeCellEditable function

var cellNode = this.activeCell.node.find('.datagrid-cell-wrapper');
var hasAlertIcon = $('.datagrid-alert-icon', cellNode).length > 0;

if (hasAlertIcon) {
this.editor.useValue = true;
}

Thank you!

@tmcconechy
Copy link
Member

Hi. On the surface that change seems like it might be ok. But im not sure where that would go. Can you maybe do a pull request with the fix and include a way to test it?

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: high team: landmark For Landmark issues type: bug 🐛 [3] Velocity rating (Fibonacci)
Projects
No open projects
Development

No branches or pull requests

4 participants