-
Notifications
You must be signed in to change notification settings - Fork 8
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
Work fe1 ljankoschek linked org UI #1834
Conversation
Pull reviewers statsStats of the last 30 days for popstellar:
|
🟠 There seems to be some formatting issues that must be applied from PR #1835 first. |
🟠 There seems to be some formatting issues that must be applied from PR #1835 first. |
1 similar comment
🟠 There seems to be some formatting issues that must be applied from PR #1835 first. |
🟠 There seems to be some formatting issues that must be applied from PR #1835 first. |
🟠 There seems to be some formatting issues that must be applied from PR #1835 first. |
1 similar comment
🟠 There seems to be some formatting issues that must be applied from PR #1835 first. |
…koschek-LinkedOrgUI Fixes by auto-format action
Please retry analysis of this Pull-Request directly on SonarCloud |
Quality Gate passed for 'PoP - Be2-Scala'Issues Measures |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really good work! Also super happy to see you adding so many tests!
I did add some comments but the PR looks quite clean overall :)
Quick question: I think I missed sth, how is the user on the UI informed that the federation succeeded?
fe1-web/src/features/linked-organizations/components/AddLinkedOrganizationButton.tsx
Outdated
Show resolved
Hide resolved
fe1-web/src/features/linked-organizations/components/AddLinkedOrganizationButton.tsx
Outdated
Show resolved
Hide resolved
fe1-web/src/features/linked-organizations/components/AddLinkedOrganizationModal.tsx
Outdated
Show resolved
Hide resolved
fe1-web/src/features/linked-organizations/components/AddLinkedOrganizationModal.tsx
Outdated
Show resolved
Hide resolved
fe1-web/src/features/linked-organizations/components/AddLinkedOrganizationModal.tsx
Outdated
Show resolved
Hide resolved
fe1-web/src/features/linked-organizations/reducer/__tests__/LinkedOrganizationsReducer.test.ts
Outdated
Show resolved
Hide resolved
fe1-web/src/features/linked-organizations/screens/LinkedOrganizationsScreen.tsx
Outdated
Show resolved
Hide resolved
@@ -305,7 +311,7 @@ namespace STRINGS { | |||
'The Roll Call is currently open and you as the organizer should start adding attendees by scanning their PoP tokens.'; | |||
export const roll_call_open_attendee = | |||
'The Roll Call is currently open and you as an attendee should let the organizer scan your PoP token encoded in the QR Code below.'; | |||
export const roll_call_qrcode_text = 'Scan to add attendee'; | |||
export const roll_call_qrcode_text = 'Scan to\nadd\nattendee'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are the newlines necessary? Did the automatic text wrapping break?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For me it didn't work here in qr code overlay
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh okay this is not good 😅 But I guess this is a good fix for now
"contentEncoding": "hex", | ||
"pattern": "^[0-9a-fA-F]{64}$", | ||
"$comment": "A 32 bytes array encoded in hexadecimal" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not that you need to change anything here, just curious: Why did you chose to go with hex instead of base64?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Honestly, when we discussed the data authentication protocol we just described to use either base64 or hex and the backend guys started to implement it with hex..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
huh okay, interesting :)
Quality Gate passed for 'PoP - PoPCHA-Web-Client'Issues Measures |
Quality Gate passed for 'PoP - Be2-Scala'Issues Measures |
Quality Gate passed for 'PoP - Be1-Go'Issues Measures |
Quality Gate passed for 'PoP - Fe1-Web'Issues Measures |
Quality Gate passed for 'PoP - Fe2-Android'Issues Measures |
UI Screens for the Federation Protocol with Tests. Added QRCode data structure and Validator.