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

Implement Servo Control Template #23981

Merged
merged 3 commits into from Aug 20, 2019
Merged

Implement Servo Control Template #23981

merged 3 commits into from Aug 20, 2019

Conversation

@paulrouget
Copy link
Contributor

paulrouget commented Aug 16, 2019

This wraps the swapchain in its own control, making it possible to better control the widget behavior.

In the future, we could package that control into a nuget package allowing any app to use Servo.

Fix #23894
Fix #23916


This change is Reviewable

@bors-servo
Copy link
Contributor

bors-servo commented Aug 17, 2019

The latest upstream changes (presumably #23978) made this pull request unmergeable. Please resolve the merge conflicts.

@paulrouget paulrouget force-pushed the paulrouget:servocontrol branch from f94e27a to 92f57b6 Aug 19, 2019
@jdm jdm assigned jdm and unassigned nox Aug 19, 2019
@Manishearth
Copy link
Member

Manishearth commented Aug 19, 2019

Verified that this patch works

Unsure if this is triggered by this patch or an earlier one, but when scrolling down my hand often scrolls beyond the content window and crosses the URL bar, and when this happens the URL entry thing gets triggered. We should probably be disabling UI control events when a touch event starts within the content window.

Copy link
Member

Manishearth left a comment

Looks great to me.

If you can figure out how to avoid scrolling and other touch-drag events from triggering the URL bar keyboard that would be great, but not necessary for this patch.

</StackPanel>
<TextBox Text="" AcceptsReturn="True" PlaceholderText="Type a URL" x:Name="urlTextbox" Grid.Column="1" IsReadOnly="True"/>
<TextBox Text="" IsTabStop="true" PlaceholderText="Type a URL" x:Name="urlTextbox" Grid.Column="1" KeyUp="OnURLEdited"/>

This comment has been minimized.

Copy link
@Manishearth

Manishearth Aug 19, 2019

Member

While you're at it could you set InputScope=Url on this? I tested it and it works, it brings up a modified keyboard.

@paulrouget
Copy link
Contributor Author

paulrouget commented Aug 20, 2019

If you can figure out how to avoid scrolling and other touch-drag events from triggering the URL bar keyboard that would be great, but not necessary for this patch.

The problem existed before, but the URL bar was only readonly, so no keyboard popping up.

In theory, this should not happen (events should be all grabbed by the focused control). I'll investigate.

@paulrouget
Copy link
Contributor Author

paulrouget commented Aug 20, 2019

Many PRs depend on this. I'll land this and file a follow-up for the focus issue.

@paulrouget
Copy link
Contributor Author

paulrouget commented Aug 20, 2019

@bors-servo r=manishearth

@bors-servo
Copy link
Contributor

bors-servo commented Aug 20, 2019

📌 Commit d096926 has been approved by @manishearth

@bors-servo
Copy link
Contributor

bors-servo commented Aug 20, 2019

💡 This pull request was already approved, no need to approve it again.

  • There's another pull request that is currently being tested, blocking this pull request: #23844
@bors-servo
Copy link
Contributor

bors-servo commented Aug 20, 2019

📌 Commit d096926 has been approved by manishearth

@highfive highfive assigned Manishearth and unassigned jdm Aug 20, 2019
@Manishearth
Copy link
Member

Manishearth commented Aug 20, 2019

I'll land this and file a follow-up for the focus issue.

sounds good, like i said, fixing the issue isn't necessary for this patch 😄

@bors-servo
Copy link
Contributor

bors-servo commented Aug 20, 2019

Testing commit d096926 with merge 17f4237...

bors-servo added a commit that referenced this pull request Aug 20, 2019
Implement Servo Control Template

This wraps the swapchain in its own control, making it possible to better control the widget behavior.

In the future, we could package that control into a nuget package allowing any app to use Servo.

Fix #23894
Fix #23916

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23981)
<!-- Reviewable:end -->
@bors-servo
Copy link
Contributor

bors-servo commented Aug 20, 2019

☀️ Test successful - linux-rel-css, linux-rel-wpt, status-taskcluster
Approved by: manishearth
Pushing 17f4237 to master...

@bors-servo bors-servo merged commit d096926 into servo:master Aug 20, 2019
3 checks passed
3 checks passed
Taskcluster (pull_request) TaskGroup: success
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
homu Test successful
Details
bors-servo added a commit that referenced this pull request Aug 20, 2019
Remove HoloLens immersivemode demo

Depends on #23981

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23982)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this pull request Aug 20, 2019
Remove HoloLens immersivemode demo

Depends on #23981

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23982)
<!-- Reviewable:end -->
bors-servo added a commit that referenced this pull request Aug 21, 2019
[hololens] Handle servo panic

Depends on #23981

Fix #23937

I've used a different approach than checking the return code of every single functions.
Basically calling a C function on panic.

That means the errors are not recoverable. I think it's fine for now.

I'm open to any other better approach.

@jdm what do you think?

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/23983)
<!-- Reviewable:end -->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

6 participants
You can’t perform that action at this time.