Re: New repo in kdereview: PlasmaTube
Hi Albert,
> qrc:/SettingsPage.qml:54: ReferenceError: logout is not defined
Fixed.
> qrc:/videoplayer/VideoControls.qml:186: TypeError: Cannot read property
> 'formatList' of undefined
Fixed.
> You're also mixing tr() and i18n() in your C++ code, please move it all to
> i18n
Should be fixed now.
> LocalizedString::setApplicationDomain("tokodon");
Fixed.
Thanks,
Devin
On Sun, Feb 5, 2023 at 4:16 AM Albert Astals Cid wrote:
>
> El dissabte, 4 de febrer de 2023, a les 19:14:20 (CET), Devin va escriure:
> > Hi everyone,
> >
> > I would like to put PlasmaTube through kdereview:
> >
> > https://invent.kde.org/multimedia/plasmatube
> >
> > PlasmaTube is a YouTube client for both mobile and desktop.
>
> These two warnings seem like you should fix them
> qrc:/SettingsPage.qml:54: ReferenceError: logout is not defined
>There's no logout anywhere
>
> qrc:/videoplayer/VideoControls.qml:186: TypeError: Cannot read property
> 'formatList' of undefined
>As far as I can see the video property of VideoControls is a MpvObject
> which doesn-t have a video property
>
>
> KLocalizedString::setApplicationDomain("tokodon");
>This application is not tokodon ;)
>
> You're also mixing tr() and i18n() in your C++ code, please move it all to
> i18n
>
>
> Cheers,
> Albert
>
>
> >
> > Thanks,
> > Devin
>
>
>
>
New repo in kdereview: PlasmaTube
Hi everyone, I would like to put PlasmaTube through kdereview: https://invent.kde.org/multimedia/plasmatube PlasmaTube is a YouTube client for both mobile and desktop. Thanks, Devin
Re: New repo in kdereview: kclock
Hi everyone, Is there any feedback left for KClock? If not, I will consider it to have passed kdereview. Thanks, Devin On Fri, Dec 30, 2022 at 2:27 AM Justin wrote: > > Thanks for the quick reply Albert. I'm not a developer but I've gone > over much of the comments from previous: > > Tested on 22.11 > > > New Timer doesn't work? Ah it's because it needs kclockd running for > that, would it be very hard to give a warning if it isn't running? > > Seems kclockd is now running automatically. If it is not automatically > running that would (in my opinion) be a packaging issue on the distro side. > > > Is that kirigamiaddons thing released? Seems to be the reason i can't > add new Alarms > > Kirigami Addons has now passed KDE review and has stable releases > > > Not an expert in UI but for stopwatch having Start/Pause on the top > left feels a bit unnatural, maybe would make sense swapping reset and > Start? Ask the VDG i guess. > > In the version I'm running it's centered nicely and I think works well. > If you can check this against latest git master to confirm the UI is > still the same. > > > Also in stopwatch for me it'd make a lot of sense if pressing the > time starts/pauses, what do you think? > > This is functioning now. > > > Silence Alarm After shows an horizontal scrollbar that doesn't to be > very useful https://i.imgur.com/7O8DBJc.png > > This is now called Ring Duration and has no horizontal scroll bar even > at the minimum window width. > > > It's even worse in Alarm Snooze Length where it goes over existing text > > Same as above, now fixed. > > > You're missing Messages.sh in kclockd > > I see this now in the kclockd folder > > > Please check i18n in your qml files, these seem like need i18n > > ... > > Lots of this i18n stuff appears to have been moved and will need a new > review > > > Justin > > On 30/12/22 20:44, Albert Astals Cid wrote: > > El divendres, 30 de desembre de 2022, a les 8:46:56 (CET), Justin va > > escriure: > >> Hey Team, > >> > >> This was last posted about on October 18 2021. I looked at the three > >> items that were needed for kirigami-addons: > >> > >> * Missing is REUSE compliance: > >> https://invent.kde.org/libraries/kirigami-addons/-/merge_requests/13 > >> * Hanyoung is working on a better time picker: > >> https://invent.kde.org/libraries/kirigami-addons/-/merge_requests/17 > >> * And Clau is working on a better date picker: > >> https://invent.kde.org/libraries/kirigami-addons/-/merge_requests/20 > >> > >> They have all now been merged. Can we resume the review on > >> kirigami-addons so we can continue on kclock? > >> > >> Thanks everyone and have a enjoyable and safe holiday! > > I gave a long list of comments back in the day that were never answered, do > > that first and maybe i give it another look. > > > > Cheers, > >Albert > > > >> Justin > > > > > >
Re: New repo in kdereview: KRecorder
> Here are my audio settings: https://i.imgur.com/8YqR82x.jpg I've adjusted the settings for vorbis visualization which *should* be better now. I think the way the visualization is calculated probably needs to be more sophisticated eventually to be smoother. > I think I found the problem; I use 11pt Noto Sans font. I can't reproduce the > issue with the default 10pt. This probably points to some width value > somewhere being a hardcoded number rather than a multiple of > Kirigami.Units.GridUnit, which hopefully should be easy to fix. I've changed it to be based on gridUnit. On Mon, Dec 5, 2022 at 4:55 PM Nate Graham wrote: > > On 12/4/22 16:47, Devin wrote: > >> I can reproduce it in the following way on Desktop: > > > > Hmm, I'm on Kirigami from master and still can't reproduce it, see the > > attached video. > > I think I found the problem; I use 11pt Noto Sans font. I can't > reproduce the issue with the default 10pt. This probably points to some > width value somewhere being a hardcoded number rather than a multiple of > Kirigami.Units.GridUnit, which hopefully should be easy to fix. > > For example: > > src/contents/ui/main.qml:25:width: Kirigami.Settings.isMobile ? 400 > : 800 > src/contents/ui/main.qml:28:property bool isWidescreen: > (appwindow.width >= 700) && appwindow.wideScreen // prevent being > widescreen at first launch > > > > >> I found one new issue that wasn't present the last time I tested: while > >> recording, the volume level indicator always indicates that it's recording > >> at max volume, even when I'm just, like, whispering: > > > > This is a hard issue to solve because different combinations of > > formats and containers produce varying audio levels, so I can't figure > > out a way to produce them properly. Perhaps once Qt 6 comes around I > > can look at the issue again since the formats will be reworked but > > perhaps what I can do is hardcode them for preset formats (currently > > all audio formats have the maximum hardcoded to 1000, which gets > > higher if the audio level exceeds that) > > Here are my audio settings: https://i.imgur.com/8YqR82x.jpg > > Like I said, this just started happening between the last time I > reviewed the app and the time before that. I didn't change any audio > settings between those times. > > > Nate
Re: New repo in kdereview: KWeather
Should be fixed now, I removed the dialog flag on the window.
Thanks,
Devin
On Tue, Nov 29, 2022 at 10:33 PM Nate Graham wrote:
>
> This happens on X11 when using QtDialogs for reasons I don't understand.
> See
> https://invent.kde.org/frameworks/knewstuff/-/commit/ea19fa6e824650f3257e8047d6f90e01899b2e03.
>
> Nate
>
>
> On 11/29/22 16:48, Devin wrote:
> > Hmm, I have no idea why the behaviour is different for you, but I get:
> > https://i.imgur.com/CvGD8HS.png
> >
> > If you removed the "Qt.Dialog" flag here:
> > https://invent.kde.org/plasma-mobile/kweather/-/blob/master/src/qml/settings/SettingsWindow.qml#L14,
> > does it still have the issue?
> >
> > Thanks,
> > Devin
> >
> > On Tue, Nov 29, 2022 at 6:44 PM Albert Astals Cid wrote:
> >>
> >> El dimecres, 30 de novembre de 2022, a les 0:33:39 (CET), Devin va
> >> escriure:
> >>>> Ok, now that i updated kirigami-addons, the Settings dialog shows, but
> >>>> it's uncloseable, any idea why that would be?
> >>> That's very strange, it should be showing up as a regular window, does
> >>> pressing the close button on the window decoration not work?
> >>
> >> Which close button?
> >>
> >> https://i.imgur.com/2jX2yUS.png
> >>
> >> Cheers,
> >>Albert
> >>
> >>>
> >>> Thanks,
> >>> Devin
> >>>
> >>> On Tue, Nov 29, 2022 at 6:23 PM Albert Astals Cid wrote:
> >>>> El dilluns, 28 de novembre de 2022, a les 17:15:57 (CET), Albert Astals
> >>>> Cid va>
> >>>> escriure:
> >>>>> El divendres, 25 de novembre de 2022, a les 4:46:40 (CET), Devin va
> >>>>
> >>>> escriure:
> >>>>>>> There's potentially overlapping text.
> >>>>>>
> >>>>>> I have fixed text wrapping in the weather delegates so it should wrap
> >>>>>> properly now.
> >>>>>>
> >>>>>>> Here settingsModel.forecastStyle is not i18n'ed
> >>>>>>
> >>>>>> I have fixed it now (it converts to i18n'd dedicated strings in the
> >>>>>> QML)
> >>>>>
> >>>>> FWIW i can't open settings anymore
> >>>>>
> >>>>> qrc:/qml/settings/SettingsWindow.qml:56:13: Type SettingsComponent
> >>>>> unavailable qrc:/qml/settings/SettingsComponent.qml:61:17: Cannot assign
> >>>>> to
> >>>>> non-existent property "onActivated" qrc:/qml/main.qml:136: TypeError:
> >>>>> Cannot call method 'close' of null
> >>>>
> >>>> Ok, now that i updated kirigami-addons, the Settings dialog shows, but
> >>>> it's
> >>>> uncloseable, any idea why that would be?
> >>>>
> >>>> Cheers,
> >>>>
> >>>>Albert
> >>>>
> >>>>> Cheers,
> >>>>>
> >>>>>Albert
> >>>>>
> >>>>>> On Mon, Nov 14, 2022 at 5:10 PM Albert Astals Cid
> >> wrote:
> >>>>>>> El dimecres, 9 de novembre de 2022, a les 23:00:13 (CET), Devin va
> >>>>
> >>>> escriure:
> >>>>>>>> Hi everyone,
> >>>>>>>>
> >>>>>>>> I would like to put kweather through kdereview:
> >>>>>>>>
> >>>>>>>> https://invent.kde.org/plasma-mobile/kweather
> >>>>>>>>
> >>>>>>>> KWeather is an application that can give simple weather
> >>>>>>>> information
> >>>>>>>> for different weather locations. Please note that KWeatherCore
> >>>>>>>> (the
> >>>>>>>> library the app depends on) has already passed kdereview.
> >>>>>>>
> >>>>>>> There's potentially overlapping text.
> >>>>>>>
> >>>>>>> See https://i.imgur.com/BrOgi3A.png
> >>>>>>>
> >>>>>>> The settings show untranslateable text, i.e.
> >>>>>>>
> >>>>>>> MobileForm.FormComboBoxDelegate {
> >>>>>>>
> >>>>>>> id: forecastStyleDropdown
> >>>>>>> text: i18n("Forecast Style")
> >>>>>>> currentValue: settingsModel.forecastStyle
> >>>>>>>
> >>>>>>> Here settingsModel.forecastStyle is not i18n'ed
> >>>>>>>
> >>>>>>> Not KWeather's fault but i just realized that Kirigami's about page
> >>>>>>> doesn't
> >>>>>>> properly credit translators :/
> >>>>>>>
> >>>>>>> Cheers,
> >>>>>>>
> >>>>>>>Albert
> >>>>>>>
> >>>>>>>> Thanks,
> >>>>>>>> Devin
> >>
> >>
> >>
> >>
Re: New repo in kdereview: KWeather
Hmm, I have no idea why the behaviour is different for you, but I get:
https://i.imgur.com/CvGD8HS.png
If you removed the "Qt.Dialog" flag here:
https://invent.kde.org/plasma-mobile/kweather/-/blob/master/src/qml/settings/SettingsWindow.qml#L14,
does it still have the issue?
Thanks,
Devin
On Tue, Nov 29, 2022 at 6:44 PM Albert Astals Cid wrote:
>
> El dimecres, 30 de novembre de 2022, a les 0:33:39 (CET), Devin va escriure:
> > > Ok, now that i updated kirigami-addons, the Settings dialog shows, but
> > > it's uncloseable, any idea why that would be?
> > That's very strange, it should be showing up as a regular window, does
> > pressing the close button on the window decoration not work?
>
> Which close button?
>
> https://i.imgur.com/2jX2yUS.png
>
> Cheers,
> Albert
>
> >
> > Thanks,
> > Devin
> >
> > On Tue, Nov 29, 2022 at 6:23 PM Albert Astals Cid wrote:
> > > El dilluns, 28 de novembre de 2022, a les 17:15:57 (CET), Albert Astals
> > > Cid va>
> > > escriure:
> > > > El divendres, 25 de novembre de 2022, a les 4:46:40 (CET), Devin va
> > >
> > > escriure:
> > > > > > There's potentially overlapping text.
> > > > >
> > > > > I have fixed text wrapping in the weather delegates so it should wrap
> > > > > properly now.
> > > > >
> > > > > > Here settingsModel.forecastStyle is not i18n'ed
> > > > >
> > > > > I have fixed it now (it converts to i18n'd dedicated strings in the
> > > > > QML)
> > > >
> > > > FWIW i can't open settings anymore
> > > >
> > > > qrc:/qml/settings/SettingsWindow.qml:56:13: Type SettingsComponent
> > > > unavailable qrc:/qml/settings/SettingsComponent.qml:61:17: Cannot assign
> > > > to
> > > > non-existent property "onActivated" qrc:/qml/main.qml:136: TypeError:
> > > > Cannot call method 'close' of null
> > >
> > > Ok, now that i updated kirigami-addons, the Settings dialog shows, but
> > > it's
> > > uncloseable, any idea why that would be?
> > >
> > > Cheers,
> > >
> > > Albert
> > >
> > > > Cheers,
> > > >
> > > > Albert
> > > >
> > > > > On Mon, Nov 14, 2022 at 5:10 PM Albert Astals Cid
> wrote:
> > > > > > El dimecres, 9 de novembre de 2022, a les 23:00:13 (CET), Devin va
> > >
> > > escriure:
> > > > > > > Hi everyone,
> > > > > > >
> > > > > > > I would like to put kweather through kdereview:
> > > > > > >
> > > > > > > https://invent.kde.org/plasma-mobile/kweather
> > > > > > >
> > > > > > > KWeather is an application that can give simple weather
> > > > > > > information
> > > > > > > for different weather locations. Please note that KWeatherCore
> > > > > > > (the
> > > > > > > library the app depends on) has already passed kdereview.
> > > > > >
> > > > > > There's potentially overlapping text.
> > > > > >
> > > > > > See https://i.imgur.com/BrOgi3A.png
> > > > > >
> > > > > > The settings show untranslateable text, i.e.
> > > > > >
> > > > > > MobileForm.FormComboBoxDelegate {
> > > > > >
> > > > > > id: forecastStyleDropdown
> > > > > > text: i18n("Forecast Style")
> > > > > > currentValue: settingsModel.forecastStyle
> > > > > >
> > > > > > Here settingsModel.forecastStyle is not i18n'ed
> > > > > >
> > > > > > Not KWeather's fault but i just realized that Kirigami's about page
> > > > > > doesn't
> > > > > > properly credit translators :/
> > > > > >
> > > > > > Cheers,
> > > > > >
> > > > > > Albert
> > > > > >
> > > > > > > Thanks,
> > > > > > > Devin
>
>
>
>
Re: New repo in kdereview: KWeather
> Ok, now that i updated kirigami-addons, the Settings dialog shows, but it's
> uncloseable, any idea why that would be?
That's very strange, it should be showing up as a regular window, does
pressing the close button on the window decoration not work?
Thanks,
Devin
On Tue, Nov 29, 2022 at 6:23 PM Albert Astals Cid wrote:
>
> El dilluns, 28 de novembre de 2022, a les 17:15:57 (CET), Albert Astals Cid va
> escriure:
> > El divendres, 25 de novembre de 2022, a les 4:46:40 (CET), Devin va
> escriure:
> > > > There's potentially overlapping text.
> > >
> > > I have fixed text wrapping in the weather delegates so it should wrap
> > > properly now.
> > >
> > > > Here settingsModel.forecastStyle is not i18n'ed
> > >
> > > I have fixed it now (it converts to i18n'd dedicated strings in the QML)
> >
> > FWIW i can't open settings anymore
> >
> > qrc:/qml/settings/SettingsWindow.qml:56:13: Type SettingsComponent
> > unavailable qrc:/qml/settings/SettingsComponent.qml:61:17: Cannot assign to
> > non-existent property "onActivated" qrc:/qml/main.qml:136: TypeError:
> > Cannot call method 'close' of null
>
> Ok, now that i updated kirigami-addons, the Settings dialog shows, but it's
> uncloseable, any idea why that would be?
>
> Cheers,
> Albert
>
> >
> > Cheers,
> > Albert
> >
> > > On Mon, Nov 14, 2022 at 5:10 PM Albert Astals Cid wrote:
> > > > El dimecres, 9 de novembre de 2022, a les 23:00:13 (CET), Devin va
> escriure:
> > > > > Hi everyone,
> > > > >
> > > > > I would like to put kweather through kdereview:
> > > > >
> > > > > https://invent.kde.org/plasma-mobile/kweather
> > > > >
> > > > > KWeather is an application that can give simple weather information
> > > > > for different weather locations. Please note that KWeatherCore (the
> > > > > library the app depends on) has already passed kdereview.
> > > >
> > > > There's potentially overlapping text.
> > > >
> > > > See https://i.imgur.com/BrOgi3A.png
> > > >
> > > > The settings show untranslateable text, i.e.
> > > >
> > > > MobileForm.FormComboBoxDelegate {
> > > >
> > > > id: forecastStyleDropdown
> > > > text: i18n("Forecast Style")
> > > > currentValue: settingsModel.forecastStyle
> > > >
> > > > Here settingsModel.forecastStyle is not i18n'ed
> > > >
> > > > Not KWeather's fault but i just realized that Kirigami's about page
> > > > doesn't
> > > > properly credit translators :/
> > > >
> > > > Cheers,
> > > >
> > > > Albert
> > > >
> > > > > Thanks,
> > > > > Devin
>
>
>
>
Re: New repo in kdereview: KWeather
> You've been told that you need to increase your minimum dependency in another > email, i'll do it again. > > Increase your required version of kirigami addons so that it matches reality. I apologize for being confused, I was wondering how I was supposed to bump a runtime QML dependency version... Nicolas added a change to add it as a proper compile time dependency now: https://invent.kde.org/plasma-mobile/kweather/-/merge_requests/79 Thanks, Devin On Mon, Nov 28, 2022 at 7:48 PM Nate Graham wrote: > > Thanks, I'm happy with the UI now! Really great app. > > Nate
Re: New repo in kdereview: KWeather
Is there some way I can do this at runtime? kirigami-addons is used here as a QML API. Thanks, Devin On Mon, Nov 28, 2022 at 12:17 PM Ingo Klöcker wrote: > > On Montag, 28. November 2022 17:53:59 CET Devin wrote: > > > FWIW i can't open settings anymore > > > > Make sure you have the latest kirigami-addons built (from master). > > I have read "use the latest kirigami-addons" many times in the last few weeks. > Isn't it possible to add a version check to detect this and show an > appropriate error if users of kirigami-addons don't use the required version? > > Whenever I add new API to libkleo that I use in Kleopatra, I bump the version > of libkleo and require the bumped version in Kleopatra, so that people > building Kleopatra immediately notice if they need to update libkleo. > > Regards, > Ingo
Re: New repo in kdereview: KWeather
> FWIW i can't open settings anymore
Make sure you have the latest kirigami-addons built (from master).
Thanks,
Devin
On Mon, Nov 28, 2022 at 11:15 AM Albert Astals Cid wrote:
>
> El divendres, 25 de novembre de 2022, a les 4:46:40 (CET), Devin va escriure:
> > > There's potentially overlapping text.
> >
> > I have fixed text wrapping in the weather delegates so it should wrap
> > properly now.
> >
> > > Here settingsModel.forecastStyle is not i18n'ed
> >
> > I have fixed it now (it converts to i18n'd dedicated strings in the QML)
>
> FWIW i can't open settings anymore
>
> qrc:/qml/settings/SettingsWindow.qml:56:13: Type SettingsComponent unavailable
> qrc:/qml/settings/SettingsComponent.qml:61:17: Cannot assign to non-existent
> property "onActivated"
> qrc:/qml/main.qml:136: TypeError: Cannot call method 'close' of null
>
> Cheers,
> Albert
>
> >
> > On Mon, Nov 14, 2022 at 5:10 PM Albert Astals Cid wrote:
> > > El dimecres, 9 de novembre de 2022, a les 23:00:13 (CET), Devin va
> > > escriure:
> > > > Hi everyone,
> > > >
> > > > I would like to put kweather through kdereview:
> > > >
> > > > https://invent.kde.org/plasma-mobile/kweather
> > > >
> > > > KWeather is an application that can give simple weather information
> > > > for different weather locations. Please note that KWeatherCore (the
> > > > library the app depends on) has already passed kdereview.
> > >
> > > There's potentially overlapping text.
> > >
> > > See https://i.imgur.com/BrOgi3A.png
> > >
> > > The settings show untranslateable text, i.e.
> > >
> > > MobileForm.FormComboBoxDelegate {
> > >
> > > id: forecastStyleDropdown
> > > text: i18n("Forecast Style")
> > > currentValue: settingsModel.forecastStyle
> > >
> > > Here settingsModel.forecastStyle is not i18n'ed
> > >
> > > Not KWeather's fault but i just realized that Kirigami's about page
> > > doesn't
> > > properly credit translators :/
> > >
> > > Cheers,
> > >
> > > Albert
> > >
> > > > Thanks,
> > > > Devin
>
>
>
>
Re: New repo in kdereview: KWeather
> There's potentially overlapping text.
I have fixed text wrapping in the weather delegates so it should wrap
properly now.
> Here settingsModel.forecastStyle is not i18n'ed
I have fixed it now (it converts to i18n'd dedicated strings in the QML)
On Mon, Nov 14, 2022 at 5:10 PM Albert Astals Cid wrote:
>
> El dimecres, 9 de novembre de 2022, a les 23:00:13 (CET), Devin va escriure:
> > Hi everyone,
> >
> > I would like to put kweather through kdereview:
> >
> > https://invent.kde.org/plasma-mobile/kweather
> >
> > KWeather is an application that can give simple weather information
> > for different weather locations. Please note that KWeatherCore (the
> > library the app depends on) has already passed kdereview.
>
> There's potentially overlapping text.
>
> See https://i.imgur.com/BrOgi3A.png
>
> The settings show untranslateable text, i.e.
>
> MobileForm.FormComboBoxDelegate {
> id: forecastStyleDropdown
> text: i18n("Forecast Style")
> currentValue: settingsModel.forecastStyle
>
> Here settingsModel.forecastStyle is not i18n'ed
>
> Not KWeather's fault but i just realized that Kirigami's about page doesn't
> properly credit translators :/
>
> Cheers,
> Albert
>
> >
> > Thanks,
> > Devin
>
>
>
>
Re: New repo in kdereview: KRecorder
> I can still see this: https://i.imgur.com/MrrwyAo.jpg No matter how I try to resize the window, I can't seem to reproduce the issue... weird > When I look at other platforms, what I see is that it's common and > traditional for the *record* button to be a red circle, but the stop button > is always a black square. And stopping the recording isn't destructive; > *deleting* the recording is destructive. I changed it to be the highlight colour, and changed the playback view as well to use the same component. > I found a new issue: when dragging the window, the view backgrounds become > partially transparent during the drag, as if I had the Translucency KWin > effect active--but I do not. In addition, after the window id dropped, its > background flickers a bit. None of my other windows do this, and KRecorder > didn't do this the last time I did it. This is strange, I did add a blur behind the window a month back, but it shouldn't be making the window transparent during drag... I don't currently experience this > Maybe when we're in widescreen mode and there are no recordings, the right > pane's placeholder message could have no action and simply say "Click the > "Record" button to make a new recording". It can probably remove the "Record > a new recording" action even when there are any recordings, since the left > pane always has a "Record" action in its header. Added On Mon, Nov 14, 2022 at 7:22 PM Nate Graham wrote: > > Much better! Most issues are fixed now. I feel like we're close. See a > few remaining comments: > > > >> The left pane's placeholder message is off-center with narrow windows. > > > > Fixed. > > I can still see this: https://i.imgur.com/MrrwyAo.jpg > > > > >> On the recording page, the "stop recording" button is red which is > >> typically our "destructive action" color. > > > > I've attempted to use other colours, as well as the selection colour, > > but they either don't contrast when flat, or don't really seem like > > buttons. > > > > I would argue that this is a destructive action, because it stops the > > recording without any way of going back. This colour is also typically > > used for stop buttons on other platforms, so I would not say that it > > is particularly out of place, in my opinion. > > When I look at other platforms, what I see is that it's common and > traditional for the *record* button to be a red circle, but the stop > button is always a black square. And stopping the recording isn't > destructive; *deleting* the recording is destructive. > > Furthermore it's inconsistent with the stop button on the playback view, > which is black: https://i.imgur.com/3u1Ogjy.jpg > > > > >> Playback buttons can get cut off with short windows: > > > > Fixed. > > I can still see this: > - https://i.imgur.com/UH9Ik8Z.jpg > - https://i.imgur.com/AnTyYyb.jpg > > > > I found a new issue: when dragging the window, the view backgrounds > become partially transparent during the drag, as if I had the > Translucency KWin effect active--but I do not. In addition, after the > window id dropped, its background flickers a bit. None of my other > windows do this, and KRecorder didn't do this the last time I did it. > > https://imgur.com/a/ZT5zJsD > > > > I continue to think the two-column view is a bit awkward when there are > no recordings, and now it results in two record actions shown at the > same time, but with different text: https://i.imgur.com/cupborn.jpg > > Maybe when we're in widescreen mode and there are no recordings, the > right pane's placeholder message could have no action and simply say > "Click the "Record" button to make a new recording". It can probably > remove the "Record a new recording" action even when there are any > recordings, since the left pane always has a "Record" action in its header. > > > Nate
Re: New repo in kdereview: KWeather
> Only nitpick I found as a user was that it's a bit weird that Settings -> > About changes the main window instead of showing about data in the little > settings window. I would suggest making About be one of the buttons on the > main window, or showing the about data in the settings window. I guess I can make the about page a popup as well, thanks! Regards, Devin On Wed, Nov 9, 2022 at 5:37 PM Jeremy Whiting wrote: > > After installing kirigami-addons it runs fine, adding my location went > smooth, etc. Only nitpick I found as a user was that it's a bit weird that > Settings -> About changes the main window instead of showing about data in > the little settings window. I would suggest making About be one of the > buttons on the main window, or showing the about data in the settings window. > > On Wed, Nov 9, 2022 at 3:31 PM Jeremy Whiting > wrote: >> >> Looks like it's got a runtime dependency on kirigami (Maybe that's expected >> for plasma-mobile, not sure) >> >> Built it on laptop and got this from cmake: >> >> cmake .. >> -- The C compiler identification is GNU 12.2.0 >> -- The CXX compiler identification is GNU 12.2.0 >> -- Detecting C compiler ABI info >> -- Detecting C compiler ABI info - done >> -- Check for working C compiler: /usr/bin/cc - skipped >> -- Detecting C compile features >> -- Detecting C compile features - done >> -- Detecting CXX compiler ABI info >> -- Detecting CXX compiler ABI info - done >> -- Check for working CXX compiler: /usr/bin/c++ - skipped >> -- Detecting CXX compile features >> -- Detecting CXX compile features - done >> -- Found Python3: /usr/bin/python3.10 (found version "3.10.8") found >> components: Interpreter >> -- Could NOT find ReuseTool (missing: REUSETOOL_EXECUTABLE) >> CMake Warning at /usr/share/ECM/modules/ECMCheckOutboundLicense.cmake:86 >> (message): >> Reuse tool not found, skipping test generation >> Call Stack (most recent call first): >> CMakeLists.txt:30 (include) >> >> >> -- Skipping execution of outbound license tests. >> -- Installing in the same prefix as Qt, adopting their path scheme. >> -- Setting build type to 'Debug' as none was specified. >> -- Looking for __GLIBC__ >> -- Looking for __GLIBC__ - found >> -- Performing Test _OFFT_IS_64BIT >> -- Performing Test _OFFT_IS_64BIT - Success >> -- Performing Test HAVE_DATE_TIME >> -- Performing Test HAVE_DATE_TIME - Success >> -- Found KF5Config: /usr/lib64/cmake/KF5Config/KF5ConfigConfig.cmake (found >> version "5.99.0") >> -- Found KF5Kirigami2: >> /usr/lib64/cmake/KF5Kirigami2/KF5Kirigami2Config.cmake (found version >> "5.99.0") >> -- Found Gettext: /usr/bin/msgmerge (found version "0.21.1") >> -- Found KF5I18n: /usr/lib64/cmake/KF5I18n/KF5I18nConfig.cmake (found >> version "5.99.0") >> -- Found KF5CoreAddons: >> /usr/lib64/cmake/KF5CoreAddons/KF5CoreAddonsConfig.cmake (found version >> "5.99.0") >> -- Found KF5Notifications: >> /usr/lib64/cmake/KF5Notifications/KF5NotificationsConfig.cmake (found >> version "5.99.0") >> -- Found KF5: success (found suitable version "5.99.0", minimum required is >> "5.90.0") found components: Config Kirigami2 I18n CoreAddons Notifications >> -- Found X11: /usr/include >> -- Looking for XOpenDisplay in /usr/lib/libX11.so;/usr/lib/libXext.so >> -- Looking for XOpenDisplay in /usr/lib/libX11.so;/usr/lib/libXext.so - found >> -- Looking for gethostbyname >> -- Looking for gethostbyname - found >> -- Looking for connect >> -- Looking for connect - found >> -- Looking for remove >> -- Looking for remove - found >> -- Looking for shmat >> -- Looking for shmat - found >> -- Looking for IceConnectionNumber in ICE >> -- Looking for IceConnectionNumber in ICE - found >> -- Found KF5Plasma: /usr/lib64/cmake/KF5Plasma/KF5PlasmaConfig.cmake (found >> version "5.99.0") >> -- Found KF5: success (found suitable version "5.99.0", minimum required is >> "5.90.0") found components: Plasma >> -- Found org.kde.kholidays-QMLModule: TRUE (found version "") >> -- The following RUNTIME packages have been found: >> >> * org.kde.kholidays-QMLModule, QML module 'org.kde.kholidays' is a runtime >> dependency. >> >> -- The following OPTIONAL packages have been found: >> >> * Python3 >> Required to run tests of module ECMCheckOutboundLicense >> * Qt5QuickCompiler >> * KF5Package (required version &g
Re: New repo in kdereview: KWeather
Hi Jeremy, Looks like I forgot to add a CMake check for Kirigami Addons. It has been added now. KWeather has a runtime dependency on Kirigami Addons as well, not just Kirigami. Thanks, Devin On Wed, Nov 9, 2022 at 5:31 PM Jeremy Whiting wrote: > > Looks like it's got a runtime dependency on kirigami (Maybe that's expected > for plasma-mobile, not sure) > > Built it on laptop and got this from cmake: > > cmake .. > -- The C compiler identification is GNU 12.2.0 > -- The CXX compiler identification is GNU 12.2.0 > -- Detecting C compiler ABI info > -- Detecting C compiler ABI info - done > -- Check for working C compiler: /usr/bin/cc - skipped > -- Detecting C compile features > -- Detecting C compile features - done > -- Detecting CXX compiler ABI info > -- Detecting CXX compiler ABI info - done > -- Check for working CXX compiler: /usr/bin/c++ - skipped > -- Detecting CXX compile features > -- Detecting CXX compile features - done > -- Found Python3: /usr/bin/python3.10 (found version "3.10.8") found > components: Interpreter > -- Could NOT find ReuseTool (missing: REUSETOOL_EXECUTABLE) > CMake Warning at /usr/share/ECM/modules/ECMCheckOutboundLicense.cmake:86 > (message): > Reuse tool not found, skipping test generation > Call Stack (most recent call first): > CMakeLists.txt:30 (include) > > > -- Skipping execution of outbound license tests. > -- Installing in the same prefix as Qt, adopting their path scheme. > -- Setting build type to 'Debug' as none was specified. > -- Looking for __GLIBC__ > -- Looking for __GLIBC__ - found > -- Performing Test _OFFT_IS_64BIT > -- Performing Test _OFFT_IS_64BIT - Success > -- Performing Test HAVE_DATE_TIME > -- Performing Test HAVE_DATE_TIME - Success > -- Found KF5Config: /usr/lib64/cmake/KF5Config/KF5ConfigConfig.cmake (found > version "5.99.0") > -- Found KF5Kirigami2: /usr/lib64/cmake/KF5Kirigami2/KF5Kirigami2Config.cmake > (found version "5.99.0") > -- Found Gettext: /usr/bin/msgmerge (found version "0.21.1") > -- Found KF5I18n: /usr/lib64/cmake/KF5I18n/KF5I18nConfig.cmake (found version > "5.99.0") > -- Found KF5CoreAddons: > /usr/lib64/cmake/KF5CoreAddons/KF5CoreAddonsConfig.cmake (found version > "5.99.0") > -- Found KF5Notifications: > /usr/lib64/cmake/KF5Notifications/KF5NotificationsConfig.cmake (found version > "5.99.0") > -- Found KF5: success (found suitable version "5.99.0", minimum required is > "5.90.0") found components: Config Kirigami2 I18n CoreAddons Notifications > -- Found X11: /usr/include > -- Looking for XOpenDisplay in /usr/lib/libX11.so;/usr/lib/libXext.so > -- Looking for XOpenDisplay in /usr/lib/libX11.so;/usr/lib/libXext.so - found > -- Looking for gethostbyname > -- Looking for gethostbyname - found > -- Looking for connect > -- Looking for connect - found > -- Looking for remove > -- Looking for remove - found > -- Looking for shmat > -- Looking for shmat - found > -- Looking for IceConnectionNumber in ICE > -- Looking for IceConnectionNumber in ICE - found > -- Found KF5Plasma: /usr/lib64/cmake/KF5Plasma/KF5PlasmaConfig.cmake (found > version "5.99.0") > -- Found KF5: success (found suitable version "5.99.0", minimum required is > "5.90.0") found components: Plasma > -- Found org.kde.kholidays-QMLModule: TRUE (found version "") > -- The following RUNTIME packages have been found: > > * org.kde.kholidays-QMLModule, QML module 'org.kde.kholidays' is a runtime > dependency. > > -- The following OPTIONAL packages have been found: > > * Python3 > Required to run tests of module ECMCheckOutboundLicense > * Qt5QuickCompiler > * KF5Package (required version >= 5.99.0) > * KF5Service (required version >= 5.99.0) > * Freetype > * PkgConfig > * Fontconfig > > -- The following REQUIRED packages have been found: > > * ECM (required version >= 5.90.0) > * Qt5Network (required version >= 5.15.7) > * Qt5QmlModels (required version >= 5.15.7) > * Qt5Quick > * Qt5Test > * Qt5Svg > * Qt5QuickControls2 > * Qt5Charts > * KF5Kirigami2 (required version >= 5.90.0) > * Gettext > * KF5I18n (required version >= 5.90.0) > * KF5Notifications (required version >= 5.90.0) > * KF5KWeatherCore (required version >= 0.6.0) > * KF5CoreAddons (required version >= 5.99.0) > * Qt5Gui (required version >= 5.15.2) > * Qt5Widgets (required version >= 5.15.2) > * KF5Plasma (required version >= 5.90.0) > * KF5 (required version >= 5.90.0) > * Qt5Qml > * Qt5Core > * Qt5 > > -- The following features have been disabled: > > * SPDX_LICENSE_TESTIN
New repo in kdereview: KWeather
Hi everyone, I would like to put kweather through kdereview: https://invent.kde.org/plasma-mobile/kweather KWeather is an application that can give simple weather information for different weather locations. Please note that KWeatherCore (the library the app depends on) has already passed kdereview. Thanks, Devin
Re: New repo in kdereview: QMLKonsole
As an update, at a recent Plasma Mobile meeting we had decided to investigate possibly using Konsole in the future (with mobile adaptations), rather than trying to continue working around qmltermwidget. However, I would still like to get this through review since we do not have any concrete timeline on doing so, and we still need a terminal emulator for Plasma Mobile for the time being. Any other feedback? Thanks, Devin On Fri, Oct 21, 2022 at 5:47 PM Devin wrote: > > > qrc:/SavedCommandsDialog.qml:44:9: QML PlaceholderMessage: Cannot specify > > left, right, and horizontalCenter anchors at the same time. > > Fixed > > > When run on X11 doesn't seem to have an icon. > > Added one explicitly in the app > > > How happy are with the name given it has "nothing" to do with konsole? > > Hmm, a new name could make sense. > > I think the original intention was to have a QML port of Konsole > eventually, but it never really panned out. qmltermwidget is a QML > port of qtermwidget, which was a fork of Konsole in the KDE 4 days :P > > > This thing has had almost 200 commits since the last release 4 years ago, > > that ain't great. Do we have any contact with the person to get them to > > release a bit more often? > > I've sent them an email, really weird that the project doesn't have a > bug tracker... > > On Fri, Oct 21, 2022 at 4:46 PM Albert Astals Cid wrote: > > > > El divendres, 21 d’octubre de 2022, a les 4:28:13 (CEST), Devin va escriure: > > > Hi everyone, > > > > > > I would like to put qmlkonsole through kdereview: > > > > > > https://invent.kde.org/plasma-mobile/qmlkonsole > > > > > > QMLKonsole is an Qt Quick based terminal emulator application for Plasma > > > Mobile. > > > > qrc:/SavedCommandsDialog.qml:44:9: QML PlaceholderMessage: Cannot specify > > left, right, and horizontalCenter anchors at the same time. > > > > When run on X11 doesn't seem to have an icon. > > > > How happy are with the name given it has "nothing" to do with konsole? > > > > > It depends on qmltermwidget: > > > https://github.com/Swordfish90/qmltermwidget > > > > This thing has had almost 200 commits since the last release 4 years ago, > > that > > ain't great. Do we have any contact with the person to get them to release a > > bit more often? > > > > It doesn't even have a issue tracker where i can report that the need to > > init > > TerminalDisplay::m_session to nullptr :/ > > > > Cheers, > > Albert > > > > > Thanks, > > > Devin > > > > > > > >
Re: New repo in kdereview: KRecorder
Hi Nate, I've done some work on addressing the feedback: > The app should have a Bugzilla component and its "Report a bug" button should > take users there, as we have been migrating towards for other mobile apps > recently. Resolved > When I open the app for the first time on the desktop, I get a > mobile-specific floating action button at the bottom of the view which > doesn't make it clear how I can record something: > > https://i.imgur.com/1xSDJkC.jpg > > On the desktop, we should not show the FAB, give the placeholder message > a "Make New Recording" action button, and then when the placeholder > message isn't visible, show a button for that action on the toolbar. > > (and IMO that would be better for mobile too) I've switched over the FAB to be a page action on widescreen. There is now a placeholder message on the default page to create a new recording. I personally think the FAB should stay on mobile, it is easier to reach and I think is fairly obvious for users that it is a record button. > When there are no recordings, the right pane is superfluous, and its message > refers to things that don't exist. The message has been updated. I think it is important to keep the two panel approach on widescreen since I think it can be disorienting for the panel setup to change when entering recordings. > The left pane's placeholder message is off-center with narrow windows. Fixed. > When I click on the Settings button, instead of the settings view > appearing as either a standalone window (as I would expect for a desktop > app) or an embedded page (as I would expect for a mobile app), it opens > as a view within a Kirigami.OverlaySheet, which is weird. Also, the use > of the new Kirigami mofile form components creates a "frames within > frames" effect that I find slightly offputting: > > https://i.imgur.com/5LN7cBJ.jpg > > When I click on any of the settings, they open in *new* OverlaySheets > which feels quite weird. And the About page does open as a real page, > but when I click the back button it doesn't take be back to where I was > before; it closes the OverlaySheet I was looking at amd I'm back on the > main view > > Overall I would recommend using full-window pages (or a standalone > window on desktop) here, rather than an OverlaySheet. I've switched over the settings dialog to be a window on widescreen. > The "Audio Quality" setting doesn't give any mention of a trade-off > between quality and file size (which I assume it at play here), leaving > me to wonder why I wouldn't always make it the highest value, and why it > doesn't default to that itself. I've added a label clarifying this. > On the recording page, the "stop recording" button is red which is typically > our "destructive action" color. I've attempted to use other colours, as well as the selection colour, but they either don't contrast when flat, or don't really seem like buttons. I would argue that this is a destructive action, because it stops the recording without any way of going back. This colour is also typically used for stop buttons on other platforms, so I would not say that it is particularly out of place, in my opinion. > The default filename for all recordings is "clip_0001", even if a file of that name already exists. Fixed. > The UI does not make it clear where files get saved to during the save > process. I took a guess and checked in ~/Music, which was correct but > that's a weird place for voice recordings which are clearly not music. > > I found the actual file path on the edit dialog, but we should show the > full file path elsewhere too, at least on the desktop where users are > more likely to care about the exact location of where files live. Added the storage location to the recording save dialog. > The "Edit" action for the individual list items should probably be renamed to > "Rename" since that's all you can do there. Same for the title of the > OverlaySheet it spawns Switched. > On the "Editing [name]" OverlaySheet, the filename field's label is "Audio > Input:" which seems inappropriate. Fixed. > Playback buttons can get cut off with short windows: Fixed. > None of the icons-only buttons on the recording or playback pages have tooltips. Fixed. Regards, Devin On Wed, Oct 26, 2022 at 11:34 AM Nate Graham wrote: > > Pretty nice app. > > > The app should have a Bugzilla component and its "Report a bug" button > should take users there, as we have been migrating towards for other > mobile apps recently. > > > Some UI review now: > > > When I open the app for the first time on the desktop, I get a > mobile-specific floating action button at the bottom of the vi
Re: New repo in kdereview: KRecorder
Any other comments or issues to address? Thanks, Devin On Fri, Oct 21, 2022 at 6:21 PM Albert Astals Cid wrote: > > El divendres, 21 d’octubre de 2022, a les 23:55:28 (CEST), Devin va escriure: > > > make install doesn't install any icon for me with the current master. > > > > I just checked and indeed, the method I changed to using > > ecm_install_icons doesn't seem to have the behaviour I thought it did. > > I hadn't verified it properly because the icon was already > > preinstalled for me. > > > > I reverted to the prior commit which installed the icon fine (it > > should be installing to > > /usr/share/icons/hicolor/scalable/apps/krecorder.svg), does this not > > work for you on X11? I double checked and the application icon shows > > for me. > > https://invent.kde.org/plasma-mobile/krecorder/-/merge_requests/17 > > Makes it work for me (when starting from the terminal) > > Cheers, > Albert > > > > > Thanks, > > Devin > > > > On Fri, Oct 21, 2022 at 5:08 PM Albert Astals Cid wrote: > > > El divendres, 21 d’octubre de 2022, a les 23:00:46 (CEST), Devin va > escriure: > > > > > The app doesn't have an icon when run in X11 > > > > > > > > Hmm, the location the icon installed to might be non-standard. I think > > > > I've fixed it on master now by copying the way other KDE apps install > > > > the icon. > > > > > > make install doesn't install any icon for me with the current master. > > > > > > Cheers, > > > > > > Albert > > > >
Re: New repo in kdereview: KRecorder
> make install doesn't install any icon for me with the current master. I just checked and indeed, the method I changed to using ecm_install_icons doesn't seem to have the behaviour I thought it did. I hadn't verified it properly because the icon was already preinstalled for me. I reverted to the prior commit which installed the icon fine (it should be installing to /usr/share/icons/hicolor/scalable/apps/krecorder.svg), does this not work for you on X11? I double checked and the application icon shows for me. Thanks, Devin On Fri, Oct 21, 2022 at 5:08 PM Albert Astals Cid wrote: > > El divendres, 21 d’octubre de 2022, a les 23:00:46 (CEST), Devin va escriure: > > > The app doesn't have an icon when run in X11 > > > > Hmm, the location the icon installed to might be non-standard. I think > > I've fixed it on master now by copying the way other KDE apps install > > the icon. > > make install doesn't install any icon for me with the current master. > > Cheers, > Albert > >
Re: New repo in kdereview: QMLKonsole
> qrc:/SavedCommandsDialog.qml:44:9: QML PlaceholderMessage: Cannot specify > left, right, and horizontalCenter anchors at the same time. Fixed > When run on X11 doesn't seem to have an icon. Added one explicitly in the app > How happy are with the name given it has "nothing" to do with konsole? Hmm, a new name could make sense. I think the original intention was to have a QML port of Konsole eventually, but it never really panned out. qmltermwidget is a QML port of qtermwidget, which was a fork of Konsole in the KDE 4 days :P > This thing has had almost 200 commits since the last release 4 years ago, > that ain't great. Do we have any contact with the person to get them to > release a bit more often? I've sent them an email, really weird that the project doesn't have a bug tracker... On Fri, Oct 21, 2022 at 4:46 PM Albert Astals Cid wrote: > > El divendres, 21 d’octubre de 2022, a les 4:28:13 (CEST), Devin va escriure: > > Hi everyone, > > > > I would like to put qmlkonsole through kdereview: > > > > https://invent.kde.org/plasma-mobile/qmlkonsole > > > > QMLKonsole is an Qt Quick based terminal emulator application for Plasma > > Mobile. > > qrc:/SavedCommandsDialog.qml:44:9: QML PlaceholderMessage: Cannot specify > left, right, and horizontalCenter anchors at the same time. > > When run on X11 doesn't seem to have an icon. > > How happy are with the name given it has "nothing" to do with konsole? > > > It depends on qmltermwidget: > > https://github.com/Swordfish90/qmltermwidget > > This thing has had almost 200 commits since the last release 4 years ago, that > ain't great. Do we have any contact with the person to get them to release a > bit more often? > > It doesn't even have a issue tracker where i can report that the need to init > TerminalDisplay::m_session to nullptr :/ > > Cheers, > Albert > > > Thanks, > > Devin > > > >
Re: New repo in kdereview: KRecorder
> Why the mov vs ogg dance? Seems to have been a bug with the saved audio format not getting applied until the settings model was initialized (in this case, when the dialog opened). It has been fixed now, thanks! > The app doesn't have an icon when run in X11 Hmm, the location the icon installed to might be non-standard. I think I've fixed it on master now by copying the way other KDE apps install the icon. > The model was kind-of-potentially an infinite tree, fixed with other small tweaks at https://invent.kde.org/plasma-mobile/krecorder/-/merge_requests/15 Thank you! Regards, Devin On Fri, Oct 21, 2022 at 4:23 PM Albert Astals Cid wrote: > > El divendres, 21 d’octubre de 2022, a les 4:25:44 (CEST), Devin va escriure: > > Hi everyone, > > > > There will be some Plasma Mobile Gear applications going through here since > > it seems some did not make it through kdereview. Our eventual goal is to > > hopefully move them to the KDE Gear release :) > > > > I would like to put krecorder through kdereview: > > > > https://invent.kde.org/plasma-mobile/krecorder > > > > KRecorder is an audio recording application for Plasma Mobile and Desktop. > > * open krecoder > * Do a recording > * gets saved as /home/tsdgeos/clip_0001.mov > * Open settings > * Close settings > * Do a recording > * gets saved as /home/tsdgeos/clip_0002.ogg > > Why the mov vs ogg dance? > > The app doesn't have an icon when run in X11 > > The model was kind-of-potentially an infinite tree, fixed with other small > tweaks at https://invent.kde.org/plasma-mobile/krecorder/-/merge_requests/15 > > Cheers, > Albert > > > > > Thanks, > > Devin > > > >
New repo in kdereview: QMLKonsole
Hi everyone, I would like to put qmlkonsole through kdereview: https://invent.kde.org/plasma-mobile/qmlkonsole QMLKonsole is an Qt Quick based terminal emulator application for Plasma Mobile. It depends on qmltermwidget: https://github.com/Swordfish90/qmltermwidget Thanks, Devin
New repo in kdereview: KRecorder
Hi everyone, There will be some Plasma Mobile Gear applications going through here since it seems some did not make it through kdereview. Our eventual goal is to hopefully move them to the KDE Gear release :) I would like to put krecorder through kdereview: https://invent.kde.org/plasma-mobile/krecorder KRecorder is an audio recording application for Plasma Mobile and Desktop. Thanks, Devin
