dasch-swiss / knora-ui

Reusable GUI elements for Knora
https://dasch-swiss.github.io/knora-ui
7 stars 1 forks source link

Improvements on the StringLiteral Input #318

Closed kilchenmann closed 5 years ago

kilchenmann commented 5 years ago

In this PR we optimized the String Literal input component from @knora/action module. And it contains a new feature: an angular pipe to stringify an array of StringLiterals:

labels: StringLiteral[] = [
  {
    value: "Frühling", 
    language: "de"
  },
  {
    value: "Spring",
    language: "en"
  },
  {
    value: "Printemps", 
    language: "fr"
  }
]

The usage in a template as {{labels | stringifyStringLiteral}} becomes Frühling (de) / Spring (en) / Printemps (fr)

flavens commented 5 years ago

@kilchenmann which repo and branch should I use for the review? Is knora-ui-playground with wip/viewer-resource ok?

kilchenmann commented 5 years ago

@flavens Oh yes I forgot to tell you in which repo I tested the component. It's wip/edit-list-nodes in Kuirl

flavens commented 5 years ago

Is it possible to add a node in a different language? Or is it part of update and it's not implemented yet? > ok I figured out that the update is not implemented and I have to fill in the input without pressing Enter in between - otherwise the edit mode is on..

flavens commented 5 years ago

So, I have spotted a few things, I do not know if it was in your specifications:

kilchenmann commented 5 years ago

Is it possible to add a node in a different language?

Yes it's possible

ok I figured out that the update is not implemented and I have to fill in the input without pressing Enter in between - otherwise the edit mode is on..

Edit nodes is disabled by using the readonly parameter on the input field. In this case the value is visible but not editable

list description displayed is the lastest submitted in the form

the different versions displayed in the placeholder is interesting but what happened if all languages are filled in and the item has got a long size?

I also thought about this possible issue. I don't know yet.

a little margin-right could be added to the select-lang button in order to give some space between the button and the input text

Yes... but the "big" button and "no margin" is only visible on button hover.

flavens commented 5 years ago

Edit nodes is disabled by using the readonly parameter on the input field. In this case the value is visible but not editable

ok! I use to press Enter once I fill in a form input, I suppose a lot of users does that. But once the field is also editable, it's no problem!

kilchenmann commented 5 years ago

ok! I use to press Enter once I fill in a form input, I suppose a lot of users does that. But once the field is also editable, it's no problem!

Yes you're right. With Enter you submit the data. I thought it's more comfortable than with a mouse click. But yes, in this case you can't add a value in a different language. Hm...that's bad and I not thought about it.

flavens commented 5 years ago

ok! I use to press Enter once I fill in a form input, I suppose a lot of users does that. But once the field is also editable, it's no problem!

Yes you're right. With Enter you submit the data. I thought it's more comfortable than with a mouse click. But yes, in this case you can't add a value in a different language. Hm...that's bad and I not thought about it.

Yes I agree, it is more comfortable. But then we need the possibility to add new value in different languages, meaning update mode

kilchenmann commented 5 years ago

Update mode will be implemented / activated as soon Knora API is ready to do so.

kilchenmann commented 5 years ago

@flavens: Can you explain this again? I do not understand anymore 😉

the list description displayed is the lastest submitted in the form (e.g. description filled in 1. EN (my user default language), 2. FR, 3. DE > the DE is displayed instead of EN)

What I did, just to explain. By creating a new list, the user starts with selecting a language for the label and adds a list label. As long the description is empty, it updates the language selector of dscription to the same as the label has. With this method, a user doesn't have to select the same language for description again (one click less). But I'm not sure if you meant this...

kilchenmann commented 5 years ago

Or did you mean the description in the list of lists?

flavens commented 5 years ago

@flavens: Can you explain this again? I do not understand anymore 😉

the list description displayed is the lastest submitted in the form (e.g. description filled in 1. EN (my user default language), 2. FR, 3. DE > the DE is displayed instead of EN)

What I did, just to explain. By creating a new list, the user starts with selecting a language for the label and adds a list label. As long the description is empty, it updates the language selector of dscription to the same as the label has. With this method, a user doesn't have to select the same language for description again (one click less). But I'm not sure if you meant this...

No, it is something different. When I create a new list, with a label and description, first in my default language (my user language, english for example) and then in German and French. The label and description displayed in the accordion menu are in German and not in English:

Screen Shot 2019-09-13 at 10 22 44
kilchenmann commented 5 years ago

Ah yes I understand. This is not part of this PR an has to be handled in dhlab-basel/Kuirl#75 There I just take the first value of the stringLiteral array. I think we need a proper StringLiteralDisplay component that shows the values in user's default language, if it exists.

flavens commented 5 years ago

Ah yes I understood. This is not part of this PR an has to be handled in #dhlab-basel/Kuirl/#75 There I just take the first value of the stringLiteral array. I think we need a proper StringLiteralDisplay component that shows the values in user's default language, if it exists.

ok I will create an issue in Kuirl and link it to the PR description

kilchenmann commented 5 years ago

@flavens I already added the comment to the existing issue. You can review this PR again, thanks.

kilchenmann commented 5 years ago

Thanks @flavens