feat: Add opacity overlay in Scan screen but keep the scan window transparent and clear #1420
Conversation
…a transparent scan window. Signed-off-by: Jian Wang <[email protected]>
Signed-off-by: Jian Wang <[email protected]>
|
|
Signed-off-by: Jian Wang <[email protected]>
Signed-off-by: Jian Wang <[email protected]>
Signed-off-by: Jian Wang <[email protected]>
Signed-off-by: Jian Wang <[email protected]>
Signed-off-by: Jian Wang <[email protected]>
Signed-off-by: Jian Wang <[email protected]>
Signed-off-by: Jian Wang <[email protected]>
Signed-off-by: Jian Wang <[email protected]>
|
The Sonar scan failed for some reason not reasonable, I think we can make an exception for this file. The clear window needs some views nesting and some tricky styles, maybe Sonar not happy with this. |
|
There was a problem hiding this comment.
Couple things to fix, but also some extra context and feedback:
The reason SonarCloud is griping about the duplication is that there is another component called NewQRView which is very similar but has many additional features and alterations. To see this other component, enable Connection Inviter Capability via developer settings and then reopen the scan screen.
The two components will eventually be merged into one and not gated behind developer mode. In the meantime we can manually dismiss the SonarCloud check on a per-PR basis, but I don't think we should add an exception into our SC config.
The change from this PR should probably be made in both components as well
| marginTop: -90, | ||
| borderWidth: 320, |
There was a problem hiding this comment.
It's best to avoid using pixel values for positioning wherever possible. Pixel values can be very brittle and have unforeseen affects across different devices, causing inconsistent UIs. Could we try to make the same change using flexbox?
There was a problem hiding this comment.
There are too many things need to be discussed, especially the NewQRView component. I pull it to draft and bring the feedbacks to team, thanks.
| }, | ||
| messageContainer: { | ||
| zIndex: 999, |
There was a problem hiding this comment.
Changing zIndex should also be avoided. It affects iOS and Android differently, and the more zIndex changes in the codebase the more likely we are to run into unexpected UI appearance and styling difficulty. In this specific case, I don't see any effect of the zIndex that couldn't be achieved with just altering the order of JSX elements.
| bottomContainer: { | ||
| position: 'absolute', | ||
| width: '100%', | ||
| bottom: 60, |
There was a problem hiding this comment.
Same as first comment above
|
@jian4on I brought this up with the rest of the BC team, and we'd like to merge the two components into one but still allow for the two variations. So the change would be making two components into one that is configured based on that connection inviter capability. In other words, there would be no change to the UI for folks using either option, but there would be less duplication. Let me know if that works for you. If it does I will tackle it ASAP so you can finish the work in this PR. 🤝 |
|
@bryce-mcmath That makes sense to do first before this PR is completed. I have not had a chance to look at the new connection inviter. Will it impact the OID4VC connection logic? |
It won't. Both components only affect UI on the scan screen, all the actual connection logic is passed through them by the same function ( |
@timbl-ont It won't. Both components only affect UI on the scan screen, all the actual connection logic is passed through them by the same function ( |
Summary of Changes
Add opacity overlay in Scan screen upon the 'ScanCamera' zone but keep the scan window clear, so that the textual contents upon the camera video could be recognized even users scan a bright background. And the clear scan window also improve the experience for QR scan alignment
Screenshots, videos, or gifs
The background image in the screenshot is only for testing, it doesn't exist in real usage.
Breaking change guide
N/A
Related Issues
N/A
Pull Request Checklist
Tick all boxes below to demonstrate that you have completed the respective task. If the item does not apply to your this PR check it anyway to make it apparent that there's nothing to do.
Signed-off-byline (we use the DCO GitHub app to enforce this)If you have any questions to any of the points above, just submit and ask! This checklist is here to help you, not to deter you from contributing!
Pro Tip 🤓