Setting readonly still allows deleting chips

[thomasdewaelheyns]

Setting readonly on the chipfield still allows deleting chips through the close button when the field is setClosable(true) and with the backspace button.
The expected behaviour would be to hide/gray out and disable the close buttons when the field is put to readonly (directly or through the binder). Also the backspace button should not allow to remove chips.

[javier-godoy]

Hello. Can you please post a small example? It seems to work as expected with

ChipField<String> chf = new ChipField<>("Read Only", "Mercury", "Venus", "Earth");
chf.setWidthFull();
chf.addSelectedItem("Mercury");
chf.addSelectedItem("Venus");
chf.setReadOnly(true);
chf.setClosable(true);

[thomasdewaelheyns]

Hi,

I don't have git installed on this laptop but with eclipse I made a simple example project that shows the behaviour I described. I think it may be more related to going from editable to readonly. I think I might have found an additional issue while making this example. If you add a new element (so one that does not exist) from the field, it will show up when you click the "Show values" button. When you remove that chip again, and press the button again, the value you created will be still in the values. The same behaviour can be observed in your Data Provider example.

To demonstrate my issues, I downloaded the master and added a new tab to the "ChipFieldDemoView". The example class is called "ReadOnlyExample" in test in the com.flowingcode.vaadin.addons.chipfield.readonly package. Please let me know if you need further input.
ChipFieldAddon_readonly.zip

[javier-godoy]

Thanks. Your example reproduces the issue, I'll investigate.

[javier-godoy]

When adding a new item, the data provider is not updated with the new item, thus findItemByLabel will not retrieve it (#35).
Additionally, setValue("Test") (called by Binder) causes the creation of a new chip that isn't registeres as an additional item with the component (#36). Then, when the component is in readonly mode, it will allow deletion of such "unknown" items.

[thomasdewaelheyns]

Are these issues being followed up on? I saw on a separate branch work was started but the last commit was 16 days ago. I'm asking because we are coming up on a test-release and I will have to substitute out the chipfield for something else otherwise while I would prefer to stick with your addon.

[javier-godoy]

Yes, and I want to thank you very much for reporting those. They helped us a lot for improving the component.

I had it in my backlog, but then I got other assignments and couldn't complete it. That branch fixes #35 and #36 which are the causes of this one. On the other hand I wasn't confident enough for a release, because the latest changes resulted in a very complicate internal logic (there are several actions, both client and server-side that result in chips being created or deleted). Then, I started with some integration tests using TestBench in order to verify there were no regressions. Would it be OK for you if we release it as an snapshot version while we wrap the changes?

[thomasdewaelheyns]

Hey Javier,

No worries, I was not meaning to rush you. Please, take the time you need to finish making this the quality component I know it can be. Since this component is not yet used by us in anything in production I can test already a snapshot version and report any issues if I encounter any. I was just asking because the inability to delete chips was impeding on testing the general workflow.

[javier-godoy]

@thomasdewaelheyns fixes were merged into 2.4.0-SNAPSHOT, which has been deployed to https://maven.flowingcode.com/snapshots

[thomasdewaelheyns]

@javier-godoy Thank you, I'll give it a go tomorrow and get back to you with my findings.

[thomasdewaelheyns]

I tested the 2.4.0-SNAPSHOT quickly and so far all issues that I noticed seem resolved.

• Changing read only state now actually locks and unlocks deleting of items correctly. On or more items can still disappear for a split second when clicking the close button or backspacing but they are correctly returned into the UI and they are not being deleted in the back-end
• Chips can now correctly be deleted from the UI and they also are correctly removed from the back-end. Both for existing chips and newly created ones.
  Next week I'm letting our research team and application specialists lose on the application and let them test more scenarios.

[javier-godoy]

Thanks for the confirmation. I'm closing this issue since the component now preserves its value in read-only mode. I'll open a separate issue about improving the visual aspect, because there are several indicators that suggest the user can interact with the component (close button, blue underline, hand cursor).

Feel free to report if your team finds some other issue, or whether this fix was not sufficient.