Skip to content
This repository was archived by the owner on Feb 19, 2024. It is now read-only.

Conversation

@Danfro
Copy link

@Danfro Danfro commented Feb 11, 2023

Fix #39 by allowing build for focal and upgrade clickable.json to clickable.yaml and update cmake minimum version to 3.5.0

@Danfro Danfro changed the title port app to focal and upgrade to clickable.yaml [draft] port app to focal and upgrade to clickable.yaml Feb 11, 2023
@Danfro
Copy link
Author

Danfro commented Feb 11, 2023

Very strange bug.

  • start recording a track
  • stop recording
  • when asked if you want to store press "save"

It does look like the tracker page does not collapse fully. But I am not sure if this is due to my device having a bigger screen than the old one or if that is a focal issue. The screen then becomes unresponsive, no action possible. Gone after restarting the app.

grafik

@mymike00
Copy link
Collaborator

Mmh that is odd... And does this happen only on focal? Maybe there is some kind of race condition or similar (maybe a timer somewhere?) that prevent the bottom edge to fully close itself...

@Danfro
Copy link
Author

Danfro commented Feb 13, 2023

Only on my focal device, yes. But that might be due to focal or maybe to the size of the screen or whatever.

Other bottom edges in this and in other apps seem to work fine. Although actually there is a similar issue with weather app when there are 10 or more stations and the user is closing station list.

Might be due to the python call? That is different.

@Danfro
Copy link
Author

Danfro commented Feb 13, 2023

Except of this flaw the app works with its main function: activity recording. Does track and print a line on screen. I did not try import/export yet.

@mymike00
Copy link
Collaborator

Only on my focal device, yes. But that might be due to focal or maybe to the size of the screen or whatever.

Other bottom edges in this and in other apps seem to work fine. Although actually there is a similar issue with weather app when there are 10 or more stations and the user is closing station list.

It seems odd that the cause would be screen size if other bottom edges work, and the fact that in weather app it depends on the number of station looks like to me even more a timing issue

Might be due to the python call? That is different.

Yeah, might be. Iirc that should only update the activity list in the home page, could you please try to comment that out and test again?

@Danfro
Copy link
Author

Danfro commented Feb 14, 2023

Your suggestion is so obvious, I wonder why I did not try that myself yet. Commenting the python call out does not solve the issue. But investigating I actually found out, that the onClicked code in ActivityDialog here and here is not needed.

Unless I have missed it, ActivityDialog is called:

In all those calls the onClicked events are overridden. So I will remove the redundant code in ActivityDialog.

@Danfro
Copy link
Author

Danfro commented Feb 14, 2023

What does actually solve the issue with the bottom edge, is to delete/comment out this code here:

newrunEdge.contentUrl = ""
newrunEdge.contentUrl = Qt.resolvedUrl("Tracker.qml")

The app seems to behave the same with this dropped. No idea what its purpose is. Should I drop it? Or do you spot a use for it?
In Main.qml the contentUrl is set the same to Tracker.qml. I see no use for setting it to "" and then setting that value again.

@Danfro
Copy link
Author

Danfro commented Feb 14, 2023

With this solved and done several recordings during the last days, I would think the draft status can be removed.

What are your plans? Will you do a xenial release with my small fixes? Then merge focal for main/master and keep development there? Or have separate focal/xenial branches to split development? Would be good to know for making maybe a few more PR's if I find time.

@Danfro Danfro changed the title [draft] port app to focal and upgrade to clickable.yaml port app to focal and upgrade to clickable.yaml Feb 14, 2023
@Danfro
Copy link
Author

Danfro commented Feb 14, 2023

Oh, and we probably should get a click out in QA group before releasing to make sure things work.

@Danfro
Copy link
Author

Danfro commented Feb 14, 2023

Another thing: before releasing anything, we need to update the pot file. I did not include that yet into the single PR's. I guess its fine to do one after everything is merged (related to focal + xenial)?

@mymike00
Copy link
Collaborator

In all those calls the onClicked events are overridden. So I will remove the redundant code in ActivityDialog.

Yeah, you are right, the onClicked code in ActivityDialog for the save button is always overridden and can be removed there, but not the one for the cancel button, as that is overridden once and the other instances use the 'default' behavior

@mymike00
Copy link
Collaborator

What does actually solve the issue with the bottom edge, is to delete/comment out this code here:

newrunEdge.contentUrl = ""
newrunEdge.contentUrl = Qt.resolvedUrl("Tracker.qml")

The app seems to behave the same with this dropped. No idea what its purpose is. Should I drop it? Or do you spot a use for it? In Main.qml the contentUrl is set the same to Tracker.qml. I see no use for setting it to "" and then setting that value again.

These two lines are meant to reset/reload the content of the bottom edge, and that is also why the url is the same as the initial one. I'm not sure why this was needed, could it be that after recording an activity and collapsing the bottom edge, if you open it again the old activity is still shown?

@mymike00
Copy link
Collaborator

I guess we could make a final xenial release and then just use the master for focal, given that there hasn't been much activity lately here too, I don't think we need (and will keep updated) a dedicated branch for xenial. Thing that we'll still be able to do in the future if needed.

Agree for updating pot file at the end, and I also would give translators some more time after merging stuff, updating pot and test everything but before publishing.

@Danfro
Copy link
Author

Danfro commented Feb 14, 2023

I'm not sure why this was needed, could it be that after recording an activity and collapsing the bottom edge, if you open it again the old activity is still shown?

Ah, you are right. I have missed that. When starting a new activity, the last type is still preselected.

I did play around with this a bit. A delay timer does not solve the freeze, only the visual error. What does seem to fix the issue, is to set newrunEdge.preloadContent = false. Since that is set to true in the onCompleted of main.qml, we should set it to true as well afterwards. This works. Although I can not see a difference if preloadContent is true or false.

@Danfro
Copy link
Author

Danfro commented Feb 17, 2023

I had a blocked UI once or twice again. I think the preload of content for bottom edge after a recorded activity is no good. I am going to give a build with that removed some test.

We should keep it in main.qml in the onCompleted event. Otherwise the map does make loading slow. But when you already did record a track, the map is cached. So no speedup there.

@lutin11
Copy link

lutin11 commented Mar 11, 2023

pot

hello, I've seen an error in pot files
msgid "Yes, Stop!" should be change to msgid "Yes, stop!" in all files

@Danfro
Copy link
Author

Danfro commented Mar 11, 2023

pot

hello, I've seen an error in pot files msgid "Yes, Stop!" should be change to msgid "Yes, stop!" in all files

The string has already been changed in the source file. The language files need to be updated. But there are other string changes too. So the plan is to merge all changes, then update the language files once.

@Danfro
Copy link
Author

Danfro commented May 5, 2023

@mymike00 would you have any time to please review & merge my PR's so we can get a clean slate (and I don't fully forget what I did) and maybe then have a first focal release?

If there is anything I can help with please let me know.

@mymike00
Copy link
Collaborator

I am really sorry for the delay but today I finally managed to spend some time on this. Thank you very much for all the work done! Only thing is that the translation may conflict with the one on weblate but I don't care that much, I'll resolve them if and when they'll occur

@mymike00 mymike00 merged commit d11dd98 into ernesst:master May 10, 2023
@Danfro
Copy link
Author

Danfro commented May 10, 2023

No worries about the delay. I left this app laying around to play with other stuff in between too.

Yes, I think translations need to be updated once all of the PR's (or at least some) are finished.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

port the app to focal

3 participants