Closed mymike00 closed 7 years ago
Can you send me a click build? I am on holiday and cant compile on my netbook...
Telegram-app build system is damn broken on my PC, therefore I am not able to say if it works as expected - neither on desktop or mobile. However I can see a few things that may need some fix.
telegram/app/qml/AccountSendMessage.qml Looks good to me. I would only ask if you could remove that 'height: units.dp(22)'. I know it was pre-existent in the code, but all that magic-numbering in telegram-app is just crazy. Units.gu(2.5) is probably a more reasonable size.
telegram/app/qml/components/AttachPanel.qml If the commented lines are no more necessary, please consider to delete them. :)
telegram/app/qml/components/AttachPanelItem.qml It looks a bit strange to me. Generally speaking, a UITK Button is not required here, as it would add more overhead. Instead, something like this might be preferable: please note 'iconName' instead of 'image' in order to match the name extensively used across UITK, and the 'backgroundColor' property in order to set background of the button. Removing EdgeShadow might improve performance a little; in any case, I consider it a nice improvement from a UI perspective and I'd like to thank you. :D
I haven't tested the code, so don't rely on me for final approval of this PR. Let's wait @Flohack74 as soon as he returns available.
Hi guys,
so far it looks OK to me, but I was not able to see the changes in a live build. Do we agree that I just merge it and we can continue to imporve it if necessary?
And here is a screenshot for it:
What about the colors @sverzegnassi maybe we find smth that fits better the Ubuntu stlye? Also the black is "too black" IMHO
@Flohack74 Those are colors from the Telegram palette. You might want to try UbuntuColors.purple, UbuntuColors.red and UbuntuColors.blue; however, colors from the Ubuntu palette are much more vibrant.
About that black - I see in the PR diff that "default" has been set as color. There is no "default" color in QML, so the QtQuick engine uses black as fallback. UbuntuColors.silk - i.e. "#CDCDCD" - might be an alternative.
@mymike00 can you review all our comments and update the PR? Would be nice to be able to release it soon ;)
Current version:
shouldn't the text be expressed in units.gu
instead of units.dp
like in line 206 (font.pixelSize: units.dp(17)
) or line 243 (font.pixelSize: units.dp(15)
) in DialogsListItem.qml
?
Well you found it out, @mymike00 the telegram code is a pure mess xD - I did not change all units now, since it would probably need try-and-error to fit them. I am not a QML guru, so I leave further improvements to the public ;)
Changed the attachment icon taking from the suru icon theme. Changed the style of the buttons to share files/photos/videos using a simple
Button
trying to use more the Ubuntu Components as mentioned in issue #31 (last point)