stadiamaps / ferrostar

A FOSS navigation SDK built from the ground up for the future
https://stadiamaps.github.io/ferrostar/
Other
180 stars 25 forks source link

Ios mute fix after discussion #296

Closed MarekSabol closed 3 weeks ago

MarekSabol commented 1 month ago

I've updated the code based on your comments. I would change one if statement after the PR 275 would be accepted. For now I used different way to check if it is navigating at the moment.

Please review the changes so far.

Closes #194

MarekSabol commented 1 month ago

I'm sorry :D, I have a development where I adjusted package and everything and try my things among the files, and second that is connected to git :D ... not the best practice but yeah, I will try to do it ASAP after work :D

MarekSabol commented 1 month ago

I will move the button to the correct file, in last commit I adjusted the code so it si way more clear, and it is using your logic "trailing" and so on. Other than moving the file is there something you don't like / I should change ? as it is simple solution for the button itself

MarekSabol commented 1 month ago

I found some problems and I will fix them today

MarekSabol commented 1 month ago

The button need the style I can't delete it.

The padding is in demonav... but it can be easily reused.

Thanks for your patience :D

MarekSabol commented 1 month ago

I have troubles to continue, I'm sorry but I cannot continue, have you ever encounter this ?

image

I dont know why it is not working I tried to implement it as a bool in all the layers and everything needed and then this happend

ianthetechie commented 1 month ago

Yes, this is a fairly normal annoyance with provisioning profiles on iOS. For a quick fix, I would change the bundle identifier to have your own prefix to be unique.

MarekSabol commented 1 month ago

tried that, been fighting with this for an hour :/ I will try to continue, hopefully I will able to manage to fix it and make this issue done

MarekSabol commented 1 month ago

I'm not paying for apple developer if thats issue

MarekSabol commented 1 month ago

I needed to rebase whole project, I have red written unknown team but it is working at the moment, thanks for hint.

I adjusted it based on the other button please let me know what you think :) it was extremely hard for me as I'm working with swift for first time ever :D .

Thanks for all of the patience

MarekSabol commented 1 month ago

at the moment it is showing all the time, but as soon as #275 will be setup we can just add if statement and it should do the work if I'm correct.

EDIT: Just checked the implementation 275, I will need to probably do something like "showZoom" but for this button to showMute or something. and map it based on navigation probably.

ianthetechie commented 1 month ago

I adjusted it based on the other button please let me know what you think :) it was extremely hard for me as I'm working with swift for first time ever :D .

Oh, wow! Jumping in the deep end with SwiftUI 😅

EDIT: Just checked the implementation 275, I will need to probably do something like "showZoom" but for this button to showMute or something. and map it based on navigation probably.

Yeah, exactly ;) It's more like showZoom than anything. You actually don't need to bother with checking if navigation is in progress, since the overlays should not be rendered in that case. By putting it with the other controls and using the grid view, that behavior comes "for free" :)

I probably won't get to a re-review this week, but will definitely have a look early next week by Tuesday.

MarekSabol commented 1 month ago

in demonav it is showing before the navigation starts :/ probably I'm missing something but maybe you will see it right away :D thanks so far :D

ianthetechie commented 1 month ago

I wouldn't worry about that. We're in the process of reworking that anyways. My goal is to have something that looks and behaves like the other optional controls (zoom buttons etc.) do today. We can handle the rest :)

ianthetechie commented 1 month ago

@Archdoog I have pushed some changes that work, and which I'm reasonably satisfied with. Would welcome a review from you though (or maybe @michaelkirk if you're interested) to sanity check.

MarekSabol commented 4 weeks ago

Hi guys, how do you run tests ? if I open only demo app, there are no tests, but if I open the whole project I have 211 errors :D, and I can't build it

ianthetechie commented 4 weeks ago

@MarekSabol for iOS tests, open the repository in Xcode, configure the scheme if necessary, and press command+u.

However, I think I know why you're getting errors locally.... As part of this PR, I've bumped the version string in Package.swift to 0.20.0, but this hasn't actually been released yet. Maybe we should stop doing that as part of PRs since it is a bit confusing.... the trouble is with how SPM is designed to look at files in git.

To get building again, either:

  1. Build locally (./build-ios.sh) and set useLocalFramework = true as described here: https://stadiamaps.github.io/ferrostar/dev-env-setup.html, OR
  2. Change the version back to 0.19.0 in your local copy
MarekSabol commented 4 weeks ago

I'm sorry if I'm annoying you :D but I'm trying I tried to build it with that, and I can't make it work. To be honest I couldn't build it even before as well in core project.

I need to open the demo app as a project in xcode, but if I open that, there are no test. I can not run any tests, not only of the part I changed.

ianthetechie commented 4 weeks ago

I'm sorry if I'm annoying you :D

No, it's no problem at all :) In fact, I'm glad that you're letting us know that the build process / instructions could be improved! That is surely discouraging other new developers too.

I need to open the demo app as a project in xcode, but if I open that, there are no test.

Correct. The demo app project does not have tests. The tests are part of the Swift package which is (perhaps confusingly) the root of the repository, because SPM requires that the Package.swift be there!

Just to double check, can you verify that your Package.swift sets the release tag to 0.19.0 like this?

image

When you first open the project and try to build, you may get an error about macros. Click on the error in the list, then click Trust & Enable at the pop-up:

image

Then you should be able to build and also to run tests. In Xcode, you'll need to configure it like this. Note that we currently use an iOS 15 simulator (we'll upgrade to 16 soon) to keep the snapshot tests stable.

MarekSabol commented 4 weeks ago

In package I had 0.17.0, I tried adjusting it to 0.19.0 and building but it didnt help

but my error looks like on the picture

image

I probably forgot some step or something :D ...

ianthetechie commented 3 weeks ago

@MarekSabol the problem in your screenshot looks like a platform issue. You've probably set it to build for macOS, but the project doesn't actually support macOS (since some critical dependencies are iOS-only).