Skip to content
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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

kenchris
Copy link

@kenchris kenchris commented Oct 14, 2019

These new elements uses material design and are build into build/elements using rollupjs.

Fixes: #14

- These new elements uses material design and are build
into build/elements using rollupjs.
Copy link
Owner

@riju riju left a 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.
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Link ?

Copy link
Author

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?

Copy link
Author

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>
Copy link
Owner

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

Copy link
Author

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) {
Copy link
Owner

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

Copy link
Author

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.
Copy link
Owner

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)

Copy link
Author

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 } }));
Copy link
Owner

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 ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@riju riju requested review from riju and removed request for riju October 17, 2019 08:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Refactor] Use a module dependency like Webpack.
2 participants