Implement first draft of additional properties for ODHActivityPoi

gappc commented 7 months ago

@RudiThoeni this is the draft PR to solve issue #549 (additional properties for ODHActivityPoi). Please do not merge yet! ;)

Example link on local machine: http://localhost:3000/dataset/edit/tourism/v1/ODHActivityPoi/echarging_it*220*ebz000043#additional-properties

The AdditionalPropertiesCell.vue is the entry point, most UI stuff is implemented in EchargingDataCell.vue and ExampleDataCell.vue.

If you want to support other additional properties, then you should take a look at types.ts and propertyOptions.ts and then add something along EchargingDataCell.vue. If you have any questions, just let me know.

There are some open topics:

as discussed, there is a list of checkboxes on the top of the page (see image1 below). The question now is what we should do if a checkbox is unchecked when the record is saved: should we remove the whole additional property? Example: the user unchecks the Echarging data checkbox, should we then remover the whole EchargingDataProperties from the AdditionalProperties?
the toggle-buttons for Show empty fields and Show deprecated fields don't work, because in this PR the Additional properties section is implemented manually (see EchargingDataCell.vue and ExampleDataCell.vue). I'll take a look at it next

image1:

RudiThoeni commented 7 months ago

Hi @gappc thx for the PR i will test it. You are right unchecking the checkbox and removing all data is not ideal.... maybe here we have to switch to a Dropdown where i can select and add to a list/ remove from list but for test the dropdown is perfect

gappc commented 7 months ago

Hey @RudiThoeni, thx for your reply. I'm not 100% sure what you mean by dropdown and list. Maybe we should schedule another call after you've performed some tests? That way we could better sort out how it should work.

RudiThoeni commented 7 months ago

@gappc Ok, let us do it this way i will have some tests first, then i will also discuss with @sseppi if we should ask to the UX experts here..... Then we will have another meeting where we discuss about how this should work the best

sseppi commented 7 months ago

@gappc Ok, let us do it this way i will have some tests first, then i will also discuss with @sseppi if we should ask to the UX experts here..... Then we will have another meeting where we discuss about how this should work the best

@RudiThoeni let me know when the new version is in testing, then we can look at the solution you implemented and decide together if we need to involve UX experts or not

gappc commented 7 months ago

@RudiThoeni @sseppi I've deployed the current version of this PR here: https://8.databrowser.gappc.net

Example: https://8.databrowser.gappc.net/dataset/edit/tourism/v1/ODHActivityPoi/echarging_it*220*ebz000043#additional-properties

RudiThoeni commented 7 months ago

@gappc thanks for deploying it, very very cool, great work! It seems very clear to me, i have to discuss with @sseppi about it..... Deleting the data when unchecking the checkbox is a little bit dangerous, maybe we have to add a button "remove" or something, i let Stefano decide if we want to involve the UX Experts.... I think this could be a very useful way to handle the dynamic property like AdditionalProperties in the future

pkritzinger commented 7 months ago

@sseppi @gappc @RudiThoeni as discussed in the weekly on Tuesday, we had a short look at it. Not sure if I get it correctly: does the feature foresee the possibility for content editors to sync matching data from the mobility-domain to the respective record? Applying the checkbox at the beginning initiates syncing?

Maybe we can have a short look at it in the next sprint meeting so that we can provide good feedback?

sseppi commented 7 months ago

@sseppi @gappc @RudiThoeni as discussed in the weekly on Tuesday, we had a short look at it. Not sure if I get it correctly: does the feature foresee the possibility for content editors to sync matching data from the mobility-domain to the respective record? Applying the checkbox at the beginning initiates syncing?

Maybe we can have a short look at it in the next sprint meeting so that we can provide good feedback?

@pkritzinger This functionality regards specific section that are related only to specific POI. The first use case was the eCharging Stations where we will in future store the time-series data in the "Mobility" domain and use then the users to input information that enrichs the description of the eCharging Stations (e.g. accessibilty information, pictures, description, human understandable name, etc.) using the "Tourism" domain and the Data Browser.

Please not that starting from January we decided that the API will be time-seres API (ex. mobility) and content API (ex- Tourism) and we will divide the usage of the two API in reference to the data type and no more with the domains.

I hope this clarifies a little more the PR. Obviously we can dedicate a slot in the next refinement meeting to this topic.

gappc commented 7 months ago

@sseppi @RudiThoeni @pkritzinger I've developed a second prototype that reuses most of the current config facilities: https://5.databrowser.gappc.net/dataset/edit/tourism/v1/ODHActivityPoi/echarging_it*220*ebz000043#additional-properties

With these changes it is now possible to define sub-menus in detail / edit view (see screenshot below). I think this could be a good alternative, because it works aligned to the current Data Browser in terms of navigation but also in terms on how it is implemented and configured by the developers.

The previously proposed solution from https://8.databrowser.gappc.net/dataset/edit/tourism/v1/ODHActivityPoi/echarging_it*220*ebz000043#additional-properties diverts more from the current concepts. It also involves more programming steps to set up a new view (in contrast to the second prototype which takes advantage of the configurations already in place)

sseppi commented 7 months ago

@gappc thank you for the new proposal, personally I like more this new proposal.

@pkritzinger what do you think?

pkritzinger commented 7 months ago

@sseppi thanks for clarifying. Maybe a short discussion in tomorrows meeting (if you need our input) would help, I did not fully understand what the usecase on user-side is. Thanks

RudiThoeni commented 7 months ago

@gappc This is very cool, i like it more than the checkbox approach because it feels more like the databrowser works....

The AdditionalProperties could be placed at the bottom of the menu and every possible Additionalproperties could be showed....

The only thing which on the checkbox approach could be handled better what came in my mind is when someone wants to "delete" Additionalproperties of a type (maybe someone inserted Echarging Props by mistake, or simple the Echarging Props are not valid anymore how can we deal with that, what do you think?

RudiThoeni commented 7 months ago

Hi @gappc @pkritzinger We discussed about the proposed solutions, we like more the approach of showing the Additionalproperties as submenu https://5.databrowser.gappc.net/dataset/edit/tourism/v1/ODHActivityPoi/echarging_it*220*ebz000043#additional-properties

What we need is a design for the Additional Properties Root -There should be the possibility to choose which Additional Property can be added (restricted to only one Additionalproperty, in future could be we need to add more than one). The List of Additional Properties to add is simply configured in the databrowser (maybe future extension could be to retrieve it from an api, but for the moment let's do it hardcoded) -Then we need also the functionality to remove an Additionalproperty. This removal will delete the content of the Additioalproperty (simply remove it from the Dictionary). This means we have to ask for confirmation.

@pkritzinger could you add a design proposal for this use case, let's discuss it in the original issue https://github.com/noi-techpark/it.bz.opendatahub.databrowser/issues/549

pkritzinger commented 5 months ago

@sseppi final version of the UI design is here: https://www.figma.com/design/DmuP6Dbv5LzkCNOXrDnWIL/2023-2024?node-id=2494-3659&t=rjZ9BJAms4N0Zyk3-1

RudiThoeni commented 2 months ago

What's the status of this PR here? For Independent would be nice to have this Visualization of the AdditionalProperties? @sseppi what do you think

sseppi commented 2 months ago

@RudiThoeni the implementation of the final version of this feature is included in the PR #596 , I think we can close this PR without merging it.

@gappc can you confirm this, or there are other features to be merged in this PR?

gappc commented 2 months ago

@sseppi we can close this PR without merging, PR #596 contains the additional properties 👍

noi-techpark / it.bz.opendatahub.databrowser

Implement first draft of additional properties for ODHActivityPoi #555