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

Implement first draft of additional properties for ODHActivityPoi #555

Draft
wants to merge 1 commit into
base: development
Choose a base branch
from

Conversation

gappc
Copy link
Collaborator

@gappc gappc commented Apr 4, 2024

No description provided.

@gappc
Copy link
Collaborator Author

gappc commented Apr 4, 2024

@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:
image

@gappc gappc changed the title Implement first draft of additional properties for ADHActivityPoi Implement first draft of additional properties for ODHActivityPoi Apr 4, 2024
@RudiThoeni
Copy link
Member

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
Copy link
Collaborator Author

gappc commented Apr 5, 2024

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
Copy link
Member

@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
Copy link
Contributor

sseppi commented Apr 5, 2024

@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
Copy link
Collaborator Author

gappc commented Apr 5, 2024

@RudiThoeni
Copy link
Member

@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
Copy link
Collaborator

@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
Copy link
Contributor

sseppi commented Apr 12, 2024

@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
Copy link
Collaborator Author

gappc commented Apr 12, 2024

@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.

image

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
Copy link
Contributor

sseppi commented Apr 12, 2024

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

@pkritzinger what do you think?

@pkritzinger
Copy link
Collaborator

@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
Copy link
Member

@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
Copy link
Member

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 #549

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants