Feature/633/custom attrs

[njaeggi]

implements #634

[lkleisa]

Huge pull request, very nice implementation!

Some UI things:

• When I want to upload a file to an encryptable, the button is now much longer than before:
  
• The validation for the custom Label is not working:
  
• There is a missing validation when the name is too long (no matter if pin, email, token):
  

[Robin481]

Nice changes but a few improvements here and there 🥉

[Robin481]

Nice changes but a few improvements here and there 🥉

P.S. this is just a code review not a functionality review, should probably do some clicking through as well

[kcinay055679]

Nice changes but a few improvements here and there 3rd_place_medal

P.S. this is just a code review not a functionality review, should probably do some clicking through as well

Marc and me resolved the requested change

[Robin481]

I think the code is getting to a good place.
I'll start with a functionality review and we can then review the code again after these changes.

On load the custom field is not aligned with the rest of the fields.

Not sure if this placeholder is intuitive to users. A quick fix would be to have it called "choose an additional field" and then grey out the text. Only make the actual options properly black. Furthermore the "Additional field" text shouldn't show up in the dropdown, doesn't make much sense.
I feel like there is a lot of usability improvement still waiting to happen. How about you don't even have the green button and just have the dropdown that says "choose an additional field" and then onClick() on one of the dropdown options it just adds said field.

If you press the green plus button in this state there should probably be a warning message like "choose one of the options in the dropdown". However, as mentioned above maybe just remove the button as it doesn't really add any value. (I know that it was in the mockup but we can be better than the mockup 😉 )

Add an on-hover message to this lock that says "this field is encrypted" and then maybe also add a crossed-out-lock to the custom attribute name that onHover says "this field is not encrypted"

The order in which the fields then are displayed is a bit confusing. I think username and password should always be the first 2 options as they are the default ones anyways. For the other ones it is fine if they are in no particular order, just not in the first or second spot.

The dropdown has a few funky interactions. One example: go to a new encryptable, add a new custom attribute by selecting it from the dropdown and pressing the green plus button. Then instantly delete the new custom attribute and press the green button again. Even though the option "Additional field" is selected it will once more add the custom attribute.

[Robin481]

Also add a little grey border around those 2 fields so you can see that they belong together.

[njaeggi]

All UI changes have been implemented, besides the new svg for non encrypted fields, everything else is resolved.

[Vakmeth]

Was noch gemacht werden muss:

• Validierung bei Form funktioniert noch nicht zu 100% => Wenn ich neue Credentials erstellen will, und nachdem ich den Accountnamen definiert habe direkt auf "submit" klicke, entsteht ein Fehler und das Frontend schmiert ab. -> Resolved by @lkleisa (Abgesprochen mit @mtnstar dürfen auch alle Felder leer sein, deshalb wurde die validation im credentials.rb für at_least_one_attribute_set entfernt)
• Auf der Pipeline failen noch 2 Tests (Systemtests), diese müssen gefixed werden -> Resolved by @lkleisa

Aufgetretene Fehler / Abgehandeltes:

• Wenn neue Credentials erstellt werden und man das Passwort eingibt, wird es nicht geblurred, dieser issue wird aber in einen anderen Ticket CSP error locally #704 behandelt.
• Das Icon (durchgestrichen) konnte eingefügt werden
• Alle Frontend Tests wurden gefixed
• Ein Border wird nur um das Custom Attributes Feld gemacht

[Robin481]

Quick feedback for @njaeggi

Bugs to fix

1. Password not hidden anymore
2. Alignment of custom field dropdown
3. Label doesn't get removed when all the fields are selected

[njaeggi]

• Offset on hover
• Not encrypted icon
• hidden password
• textli

[Robin481]

Seems fine to me! 🚀 The UI is now quite nice if you ask me. 🥳
However, let @mtnstar also review it one more time.

[Vakmeth]

Momentaner Stand:

• Eine letzte Änderung wurde von mir durchgeführt, der Pullrequest wurde von Robin reviewed und dieser hat ihn als "fine" eingestuft.
• Nächster Schritt: Zur doppelten Überprüfung muss noch @mtnstar einen Blick über den Pullrequest werfen, danach kann der Branch gemerged werden.

[mtnstar]

on encryptables#show:

styling? make sure all attribute boxes and copy controls look the same and are aligned. maybe give them more width?
or use two columns?

maybe we could integrate the copy buttons inside the read-only input field? https://getbootstrap.com/docs/5.3/forms/input-group/#button-addons

[mtnstar]

same as above for the list view ...

maybe we could align the value/copy boxes to the right?

[mtnstar]

on encryptables#show ... password is shown in clear text on my local dev machine.

should be with the secret font ... maybe I just missed to install something?

UPDATE: oh I just saw, CSP is blocking it on my machine. if it's only a problem on my machine, just ignore it

[mtnstar]

is there a reason why there's a blank value inside this drop-down? can't we just pre-select any of the options and the user might change it?