Initial `NavigationView` implementation #130

j-f1 · 2020-06-30T19:09:23Z

Fixes #129.

Current status:

Displays the navigation links
Accessible
NavigationLink is clearly clickable
Destination is visible after clicking

j-f1 · 2020-07-01T17:19:00Z

Decided to try to refactor the Proxy structs into one shared type but ran into feedback-assistant/reports#128 ¯\_(ツ)_/¯

j-f1 · 2020-07-01T18:08:52Z

It looks like SwiftWebUI uses Combine to handle the NavigationView. Is it OK to add that as a dependency?

MaxDesiatov · 2020-07-01T19:13:20Z

Yeah, sure, I have a version here tailored for SwiftWasm just for this occasion 😉

MaxDesiatov · 2020-07-01T19:19:23Z

Also going to be handy for things such as #141

j-f1 · 2020-07-01T19:59:09Z

Turns out I didn’t end up needing it anyway! Simply overwriting the wrappedValue of a Binding is sufficient.

j-f1 · 2020-07-02T00:10:20Z

I merged in main and now I’m getting a crash:

[Error] Unhandled Promise Rejection: RuntimeError: Out of bounds memory access (evaluating 'a._start()')

Stack

	<?>.wasm-function[searchInConformanceCache(swift::TargetMetadata<swift::InProcess> const*, swift::TargetProtocolDescriptor<swift::InProcess> const*)]
	<?>.wasm-function[swift_conformsToSwiftProtocolImpl(swift::TargetMetadata<swift::InProcess> const*, swift::TargetProtocolDescriptor<swift::InProcess> const*, __swift::__runtime::llvm::StringRef)]
	<?>.wasm-function[swift_conformsToProtocolImpl(swift::TargetMetadata<swift::InProcess> const*, swift::TargetProtocolDescriptor<swift::InProcess> const*)]
	<?>.wasm-function[swift_conformsToProtocol]
	<?>.wasm-function[_conformsToProtocols(swift::OpaqueValue const*, swift::TargetMetadata<swift::InProcess> const*, swift::TargetExistentialTypeMetadata<swift::InProcess> const*, swift::TargetWitnessTable<swift::InProcess> const**)]
	<?>.wasm-function[_dynamicCastToExistential(swift::OpaqueValue*, swift::OpaqueValue*, swift::TargetMetadata<swift::InProcess> const*, swift::TargetExistentialTypeMetadata<swift::InProcess> const*, swift::DynamicCastFlags)]
	<?>.wasm-function[swift_dynamicCastImpl(swift::OpaqueValue*, swift::OpaqueValue*, swift::TargetMetadata<swift::InProcess> const*, swift::TargetMetadata<swift::InProcess> const*, swift::DynamicCastFlags)]
	<?>.wasm-function[swift_dynamicCast]
	<?>.wasm-function[$s11TokamakCore7AnyViewVyACxcAA0D0RzlufcACypcfU_]
	<?>.wasm-function[$s11TokamakCore15StackReconcilerC6render13compositeViewQrAA016MountedCompositeG0CyxG_tF]
	<?>.wasm-function[$s11TokamakCore20MountedCompositeViewC5mount4withyAA15StackReconcilerCyxG_tF]
	<?>.wasm-function[$s11TokamakCore15MountedHostViewC5mount4withyAA15StackReconcilerCyxG_tFyAA0cE0CyxGXEfU0_]
	<?>.wasm-function[$s11TokamakCore15MountedHostViewC5mount4withyAA15StackReconcilerCyxG_tFyAA0cE0CyxGXEfU0_TA]
	<?>.wasm-function[$s11TokamakCore11MountedViewCyxGs5Error_pIggzo_ADsAE_pIegnzo_AA8RendererRzlTR]
	<?>.wasm-function[$s11TokamakCore11MountedViewCyxGs5Error_pIggzo_ADsAE_pIegnzo_AA8RendererRzlTRTA]
	<?>.wasm-function[$sSTsE7forEachyyy7ElementQzKXEKF]
	<?>.wasm-function[$s11TokamakCore15MountedHostViewC5mount4withyAA15StackReconcilerCyxG_tF]
	<?>.wasm-function[$s11TokamakCore20MountedCompositeViewC5mount4withyAA15StackReconcilerCyxG_tF]
	<?>.wasm-function[$s11TokamakCore15StackReconcilerC4view6target8renderer9schedulerACyxGqd___10TargetTypeQzxyyycctcAA4ViewRd__lufc]
	<?>.wasm-function[$s11TokamakCore15StackReconcilerC4view6target8renderer9schedulerACyxGqd___10TargetTypeQzxyyycctcAA4ViewRd__lufC]
	<?>.wasm-function[$s10TokamakDOM11DOMRendererCyACx_13JavaScriptKit11JSObjectRefCtc0A4Core4ViewRzlufc]
	<?>.wasm-function[$s10TokamakDOM11DOMRendererCyACx_13JavaScriptKit11JSObjectRefCtc0A4Core4ViewRzlufC]
	<?>.wasm-function[main]
	<?>.wasm-function[__original_main]
	<?>.wasm-function[_start]
	wasm-stub
	_start
	(anonymous function) (index.esm.js:182:184)
	(anonymous function) (dev.js:64)
	asyncFunctionResume
	(anonymous function)
	promiseReactionJobWithoutPromise
	promiseReactionJob

MaxDesiatov · 2020-07-02T00:19:50Z

This might be a SwiftWasm bug, I'd recommend trying with more recent (or older) snapshots, but the most recent snapshots broke SwiftPM on macOS as far as I remember. The process of picking a correct snapshot probably is going to be tedious, so maybe it would be better to wait until I get most recent snapshots fixed, IDK 🤔

MaxDesiatov · 2020-07-02T00:20:50Z

Also, IDK if there actually is a snapshot of SwiftWasm that doesn't have this broken, would be great to isolate what exact code is actually causing this.

Sources/TokamakDOM/Views/NavigationLink.swift

j-f1 · 2020-07-03T03:43:28Z

Bug remains present on the latest SwiftWasm wasm-DEVELOPMENT-SNAPSHOT-2020-07-02-a.

Overall, repros on:

wasm-DEVELOPMENT-SNAPSHOT-2020-07-02-a
wasm-DEVELOPMENT-SNAPSHOT-2020-06-12-a
wasm-DEVELOPMENT-SNAPSHOT-2020-06-02-a

It fails to build on wasm-5.3-SNAPSHOT-2020-07-02-a with error: 'stdlib.h' file not found.

MaxDesiatov · 2020-07-03T11:05:50Z

Oh, interesting, my understanding was that SwiftPM is still broken for wasm-DEVELOPMENT-SNAPSHOT-2020-07-02-a on macOS, or did you try that on Linux?

j-f1 · 2020-07-03T12:24:34Z

I tried it on macOS. What do you mean be SwiftPM being broken?

MaxDesiatov · 2020-07-03T14:36:04Z

I was relying on the CI logs in swiftwasm/swift#1233 in the "Run smoke tests on macOS" step, which fails with a weird error I didn't have time yet to look into. But I'm happy that it actually works, means I only need to fix the way it's used on CI, I guess.

j-f1 · 2020-07-03T15:26:31Z

I sometimes get weird errors but sometimes it works just fine ¯\_(ツ)_/¯

MaxDesiatov · 2020-07-03T15:28:46Z

Please report the errors to the main swiftwasm/swift project, if you don't mind. I guess nuking the .build/wasm32-unknown-wasi directory may produce a cleaner environment for reproduction.

j-f1 · 2020-07-03T15:44:41Z

swiftwasm/swift#1362 & swiftwasm/swift#1363

Sources/TokamakCore/Views/NavigationView.swift

MaxDesiatov · 2020-07-20T20:57:07Z

It’s getting chopped off because box-sizing is not set to border-box or padding-box.

Sorry, I'm not sure if this PR is fully ready for review then, and if not, do you plan to address this, or maybe land that as a separate PR?

j-f1 · 2020-07-20T21:00:28Z

That’s a separate issue since it also reproduces on main it’s just less obvious because it stretches all the way across the screen.

This reverts commit 73c9c3f.

MaxDesiatov

Great stuff, thank you so much 👏

j-f1 · 2020-07-20T21:19:10Z

Did some testing and it looks like the @ViewBuilder tricks you suggested crash with an EXC_BAD_ACCESS (code=1, address=0x0) on iOS 13/macOS 10.15. Do you want to keep support for them for now or is it ok to require 10.16/14 betas?

MaxDesiatov · 2020-07-20T21:27:18Z

Probably ok to require them, unless you find it more convenient to develop and test on iOS 13/macOS 10.15. I don't have Big Sur installed and don't intend to until it's stable enough, and testing with an iOS 14 simulator is fine for me personally. @carson-katri what do you think?

carson-katri · 2020-07-20T21:58:30Z

I have a machine with Big Sur, so no problems here.

j-f1 · 2020-07-20T22:02:44Z

I’d rather have things working on my existing devices (13/10.15) and the AnyView stuff isn’t a huge issue.

MaxDesiatov

This is great stuff and looks good to me 👍 Apologies for all the delays, but would you be able to resolve the conflicts?

MaxDesiatov

Thanks 🙌

MaxDesiatov · 2020-07-22T20:59:59Z

Conflicts 🙈

carson-katri · 2020-07-22T21:01:57Z

I resolved it. Sorry, I merged App protocol before this, that's on me.

`Toolbar` is new in SwiftUI. It is coupled fairly closely with `NavigationView`, so this should be integrated with that somehow (#130). It was made similar to macOS which allows more than a leading/trailing `ToolbarItem`. Resolves #316.

Initial NavigationView implementation

93456b1

j-f1 force-pushed the navigation branch from fe14a6c to 93456b1 Compare June 30, 2020 19:11

j-f1 added the SwiftUI compatibility Tokamak API differences with SwiftUI label Jun 30, 2020

j-f1 marked this pull request as draft June 30, 2020 19:12

Merge branch 'main' into navigation

a07e90a

j-f1 added 2 commits July 1, 2020 13:24

Make the _ButtonProxy type more generic

364651b

Split Navigation.swift files

94d523d

j-f1 added 2 commits July 1, 2020 15:29

Move this too

36f45ad

Implement Navigation controls

336b1b6

Merge branch 'main' into navigation

d4825ea

j-f1 added 2 commits July 2, 2020 13:23

Update progress.md

ce22d0f

Merge branch 'main' into navigation

8738b41

MaxDesiatov reviewed Jul 2, 2020

View reviewed changes

Sources/TokamakDOM/Views/NavigationLink.swift Outdated Show resolved Hide resolved

Make NavigationLinks links

5f829a4

Merge branch 'main' into navigation

f26b46b

MaxDesiatov reviewed Jul 20, 2020

View reviewed changes

Sources/TokamakCore/Views/NavigationView.swift Outdated Show resolved Hide resolved

j-f1 added 3 commits July 20, 2020 17:09

Switch default destination to EmptyView

e4b60d8

Fix build for macOS

3721737

Revert "Apply suggestions from @MaxDesiatov"

538994e

This reverts commit 73c9c3f.

MaxDesiatov requested a review from carson-katri July 20, 2020 21:15

MaxDesiatov previously approved these changes Jul 20, 2020

View reviewed changes

j-f1 dismissed MaxDesiatov’s stale review via 538994e July 20, 2020 22:03

MaxDesiatov mentioned this pull request Jul 21, 2020

0.2.0 release #131

Closed

4 tasks

MaxDesiatov reviewed Jul 22, 2020

View reviewed changes

Merge branch 'main' into navigation

edf0374

MaxDesiatov previously approved these changes Jul 22, 2020

View reviewed changes

MaxDesiatov changed the title ~~Initial NavigationView implementation~~ Initial NavigationView implementation Jul 22, 2020

carson-katri previously approved these changes Jul 22, 2020

View reviewed changes

Merge branch 'main' into navigation

46ab7b9

carson-katri dismissed stale reviews from MaxDesiatov and themself via 46ab7b9 July 22, 2020 21:07

carson-katri self-requested a review July 22, 2020 21:09

carson-katri approved these changes Jul 22, 2020

View reviewed changes

MaxDesiatov approved these changes Jul 22, 2020

View reviewed changes

j-f1 merged commit ac50208 into main Jul 22, 2020

j-f1 deleted the navigation branch July 22, 2020 21:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Initial `NavigationView` implementation #130

Initial `NavigationView` implementation #130

j-f1 commented Jun 30, 2020 •

edited

j-f1 commented Jul 1, 2020

j-f1 commented Jul 1, 2020

MaxDesiatov commented Jul 1, 2020

MaxDesiatov commented Jul 1, 2020

j-f1 commented Jul 1, 2020

j-f1 commented Jul 2, 2020

MaxDesiatov commented Jul 2, 2020

MaxDesiatov commented Jul 2, 2020

j-f1 commented Jul 3, 2020 •

edited

MaxDesiatov commented Jul 3, 2020

j-f1 commented Jul 3, 2020

MaxDesiatov commented Jul 3, 2020

j-f1 commented Jul 3, 2020

MaxDesiatov commented Jul 3, 2020

j-f1 commented Jul 3, 2020

MaxDesiatov commented Jul 20, 2020

j-f1 commented Jul 20, 2020

MaxDesiatov left a comment

j-f1 commented Jul 20, 2020

MaxDesiatov commented Jul 20, 2020 •

edited

carson-katri commented Jul 20, 2020

j-f1 commented Jul 20, 2020

MaxDesiatov left a comment •

edited

MaxDesiatov left a comment

MaxDesiatov commented Jul 22, 2020

carson-katri commented Jul 22, 2020 •

edited

Initial NavigationView implementation #130

Initial NavigationView implementation #130

Conversation

j-f1 commented Jun 30, 2020 • edited

j-f1 commented Jul 1, 2020

j-f1 commented Jul 1, 2020

MaxDesiatov commented Jul 1, 2020

MaxDesiatov commented Jul 1, 2020

j-f1 commented Jul 1, 2020

j-f1 commented Jul 2, 2020

MaxDesiatov commented Jul 2, 2020

MaxDesiatov commented Jul 2, 2020

j-f1 commented Jul 3, 2020 • edited

MaxDesiatov commented Jul 3, 2020

j-f1 commented Jul 3, 2020

MaxDesiatov commented Jul 3, 2020

j-f1 commented Jul 3, 2020

MaxDesiatov commented Jul 3, 2020

j-f1 commented Jul 3, 2020

MaxDesiatov commented Jul 20, 2020

j-f1 commented Jul 20, 2020

MaxDesiatov left a comment

Choose a reason for hiding this comment

j-f1 commented Jul 20, 2020

MaxDesiatov commented Jul 20, 2020 • edited

carson-katri commented Jul 20, 2020

j-f1 commented Jul 20, 2020

MaxDesiatov left a comment • edited

Choose a reason for hiding this comment

MaxDesiatov left a comment

Choose a reason for hiding this comment

MaxDesiatov commented Jul 22, 2020

carson-katri commented Jul 22, 2020 • edited

Initial `NavigationView` implementation #130

Initial `NavigationView` implementation #130

j-f1 commented Jun 30, 2020 •

edited

j-f1 commented Jul 3, 2020 •

edited

MaxDesiatov commented Jul 20, 2020 •

edited

MaxDesiatov left a comment •

edited

carson-katri commented Jul 22, 2020 •

edited