-
Notifications
You must be signed in to change notification settings - Fork 60
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
Create camera-capture element and an element for logging #35
base: master
Are you sure you want to change the base?
Conversation
- These new elements uses material design and are build into build/elements using rollupjs.
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.
Thank you! The UX makeover looks cool ;) let fix some nits.
} | ||
}); | ||
|
||
// Android bug, doesn't work with DevTools emulation. |
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.
Link ?
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 filed, but can you look at it and debug with Sasha?
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.
She had a similar workaround that broke devtools emulation
<settings-slider metering label="Focus distance" id="focusDistance" min="0" max="10" value="0" step="1"></settings-slider> | ||
</div> | ||
<div id="standardSettings" class="pro-settings hidden"> | ||
<settings-slider label="Contrast" id="contrast" min="0" max="10" value="0" step="1"></settings-slider> |
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.
remove defaults? we are reading it in L.108
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.
Done
|
||
facingMode = "user"; | ||
|
||
_onResetClicked(e) { |
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.
How about reset-ing state per settings group ? Like it was before, so that we have more fine grained control of things we are resetting
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.
Done
this.constraints.height = Math.ceil(visualViewport.height); | ||
} | ||
|
||
// Disable facingModeButton if there is no environment or user mode. |
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.
Let's elaborate a bit more on the comment, as we discussed ? Do we want to treat that icon as a camera switcher (say 2 user facing mode cameras on a laptop, the inbuilt and an external one), or a facing mode switcher like say on Android, (front/user and back/environment)
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.
Switching camera on a mobile phone with just two cameras is the exact same behavior as switching between front and back
constraints.advanced[0]['whiteBalanceMode'] = 'manual'; | ||
} | ||
|
||
this.dispatchEvent(new CustomEvent('constraintschange', { detail: { constraints } })); |
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.
Now the constraintschange
change event is fired on mouse up, previously we changed it instantly on slider value change. Can we change the functionality to what it was ?
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.
done
These new elements uses material design and are build into build/elements using rollupjs.
Fixes: #14