Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Suggestion] Optimise layout of MeeCast's Event-View #75

Open
FingusDE opened this issue Nov 22, 2024 · 21 comments
Open

[Suggestion] Optimise layout of MeeCast's Event-View #75

FingusDE opened this issue Nov 22, 2024 · 21 comments
Assignees

Comments

@FingusDE
Copy link

Missing Column in Event-View on Jolla C2, Jolla Tablet, Redmi 5 Plus

bd9fcef7eabc9d25874fbc973c8b7be91328d365

My suggestion is to stretch it to full width or add another day as column to it. The same for the Landscape-Display.

@FingusDE FingusDE added the bug label Nov 22, 2024
@Olf0 Olf0 changed the title [Bug] Missing Column in Event-View on Jolla C2, Jolla Tablet, Redmi 5 Plus in Portrait-Mode [Suggestion] Optimise layout of MeeCast's Event-View Nov 22, 2024
@Olf0 Olf0 added enhancement and removed bug labels Nov 22, 2024
@Olf0
Copy link
Member

Olf0 commented Nov 22, 2024

@FingusDE, thank you very much for filing this issue here. I categorised this as a suggestion, because it aims at beautifying the look, but everything is already fully functional.

As already denoted, IMO there is no space left for another icon (mind, that some empty space to the right screen border must be kept): The number of icons is currently 4 in portrait and 8 in landscape orientation, as you can easily see when you turn your device from portrait to landscape orientation.

To stretch this row of icons should be technically possible (with QML / Qt), but this is an "expensive" operation, i.e. it is slow and battery consuming.

IMO the elements of MeeCast's Event-View should be arranged as follows to look better:

  1. Create a centred column with equal margins (free space) to the left and right, and the necessary margins to the top and below (the latter may be 0, IDK).
  2. Place the icon displaying the current weather in the top left corner (i.e. left- and top-aligned, with no extra margin) of this column.
  3. Place the location (here: Havixbeck) centred in the column, with no or very little margin at its top.
    Use the same text element for the textual representation of the current weather (here: Bedeckt) in a smaller font size (the current one is fine, IMO) and separated by a line break (it is likely already implemented this way).
  4. Centre the row of weather forecast icons in the box, with a little margin at its top. IMO, currently there is slightly too little margin between the textual representation of the current weather and the icon-row (which is more pronounced in my screenshots below). Maybe this can be implemented as part of the element described in 3.
  5. Right-align the status message for the last poll time: IMO there is nothing to do, it currently looks excellent. But I wonder, why the left and right margin (of the column) seem to differ: The right margin appears to be almost double as wide as the left margin.

Side note: I wrote down these considerations before looking at the QML code: Many aspects are already implemented this way, and my considerations are better adapted to the current code-base than the other way around. Main point of my considerations is: This is how I imagine a better look, I do not care how this is achieved.

Other ideas for optimising the layout of MeeCast's Event-View are welcome, even more so a Pull Request which performs these optimisations. The corresponding QML code is here: sailfishos/meecast/event-components/WeatherBanner.qml

P.S.: Screenshots of the current look of MeeCast's Event-View v1.11.7 on an Sony Xperia X with SailfishOS 3.2.1:

  • In landscape orientation
    meecast-eventview_xperia-x_landscape
  • In portrait orientation
    meecast-eventview_xperia-x_portrait

@pagism
Copy link

pagism commented Nov 23, 2024

The old Jolla weather app was displaying 5 icons in portrait mode. Personally I think 5 icons makes more sense than 4, especially when on the right there is a noticeable gap. Also in horizontal view I would prefer to have 7 (week) instead of 8. Maybe having the size of icons smaller will fit a 5th icon?

In addition on 10iii the font size of the first line of the weather location does not match the rest of the event screen especially when the date is also displayed at the top.

@Olf0
Copy link
Member

Olf0 commented Nov 24, 2024

Maybe having the size of icons smaller will fit a 5th icon?

It sure would, but I have not seen vector graphic sources (e.g. in SVG format) of these icons (maybe I missed them). Scaling bitmap icons (e.g. PNG format) results in either ugly or blurred results (no matter if they are already scaled embedded in MeeCast or if MeeCast lets Qt scale them).

But that is a different topic than rectifying the visual layout of MeeCast's Eventview: Making the left and right borders equally large, plus centring the city name, textual weather indicator and the icon row.

Also in horizontal view I would prefer to have 7 (week) instead of 8.

I do not concur, I want as many as possible with a given icon size.

@pagism
Copy link

pagism commented Nov 24, 2024

I am not a GUI expert, the jolla weather app event view layout is good and simple imo.

The top line is centered, symbol size and location fonts are larger than other text lines around.

Then for the second line it looks like each day-cell is split vertically to 4 rows, i.e. day, temp max, symbol, temp min. This allows to reduce the width of that cell to the icon size only. It does not have a fixed number of days, depending or screen resolution/scale, it can range from 4-6, and I guess on landscape will fit as many days as that horizontal space allows.

I do not know much about the icon graphics, but I get the impression that some of those sets have too much detail that might not help with scaling.

@Olf0
Copy link
Member

Olf0 commented Nov 25, 2024

I am not a GUI expert, the jolla weather app event view layout is good and simple imo.

965f3f3850a6973f6f8800166d258e6fa86f0a94_jolla-weather-app

This is definitely looking good. (Side note: I never used Foreca's proprietary weather app due to privacy; AFAIR it transmits the device's location to Foreca, Foreca is the only weather data provider available and Foreca's forcast data is in my experience less reliable than weather.com.)
But as MeeCast has only a single maintainer who has very little time (I am merely supporting him with infrastructure work), any major redesign is out of scope, unless someone (you?) contributes it.

Hence, lets us focus on slightly rearranging the extant icons and look (which I actually like a bit better than Foreca's), so it does not look awkward any longer. Even that might require a contribution, as I do not have any time for that this year and I doubt @vasvlad can spare any either for this; ultimately it is just a beautification, the functionality is there (i.e. nothing is broken: it is a feature-request, not a bug).

@panositi
Copy link

@Olf0 yes it's an improvement not a bug. I have no experience in QML but I will give it a try when I get more time to setup the dev environment and will let you know.

@Olf0
Copy link
Member

Olf0 commented Nov 25, 2024

I have no experience in QML but I will give it a try when I get more time to setup the dev environment and will let you know.

The nice and convenient thing about QML / QtQuick is that it is just JavaScript using Qt functions, and the excellent documentation by the Qt company and also third parties.

  • As it is JavaScript, i.e. an interpreted language, there is no need for a developer environment: Simply edit WeatherBanner.qml on your device (should be at /usr/lib/qt5/qml/Sailfish/Weather/WeatherBanner.qml; much more conveniently when logging in to your device via SSH) and restart lipstick (systemctl --user restart lipstick), then you will see the effect of your changes.
  • Mind to use the documentation for Qt 5.6: By the Qt company, its sub-page for QML, and the functions reference at devdocs.io which provides specific sections for "Qt QML" and "Qt Quick".

The inconvenient thing about QML is that it is JavaScript using Qt functions, for me: I hate to code in JavaScript, never became accustomed to it and I am not proficient in using Qt.

@pagism
Copy link

pagism commented Nov 26, 2024

I am almost ignorant in QML and I never liked javaScript either lol. Anyway, it's really slow trial and error process here. I managed to fix the top line, the second one is more complicated as requires a very fine placement and alignment of items which I do not understand yet.

The reason we have that gap on the right is that alignment starts from left and for portrait and landscape mode the number of days is fixed to 4 and 8 respectively. For higher resolution screens (?) that is not enough to fill the entire line hence the gap.

@Olf0
Copy link
Member

Olf0 commented Nov 26, 2024

I am almost ignorant in QML and I never liked javaScript either lol. Anyway, it's really slow trial and error process here. I managed to fix the top line, the second one is more complicated as requires a very fine placement and alignment of items which I do not understand yet.

I feel with you: That reflects exactly my experiences with QML. It should be "so easy" (maybe it is for web-page programmers accustomed to JavaScript), the Qt documentation is really well done, but I always struggle hard. Some "quick wins" are indeed easy to achieve, but in order to obtain a QML page layout as I imagined it, I ultimately struggle.

The reason we have that gap on the right is that alignment starts from left and for portrait and landscape mode the number of days is fixed to 4 and 8 respectively. For higher resolution screens (?) that is not enough to fill the entire line hence the gap.

Yes, exactly. I always wondered if this effect is really (or "really only") dependent on the screen-resolution, especially as QML should (just as HTML) be resolution-independent.

Hence I believe it is a wrong approach trying to adjust the layout depending on the screen resolution.
Thus my suggestion to keep it simple and just centre the row with the (4 or 8) weather forecast icons horizontally in order to rectify the look.

@vasvlad
Copy link
Member

vasvlad commented Nov 27, 2024

Hi.
I'm glad to see this discussion. My code (QML-part) for Event-View is not optimal (and most likely ugly). I coded it very quickly when Jolla had only one device(and one display resolution) I hope I'll find time to refactor it. @pagism please send patch or pull request of your current work.

@pagism
Copy link

pagism commented Nov 28, 2024

I am struggling with the second line margins, slow process on that, I will send you my patch once it looks ok.

Is three anyway to find out whats in silica.weather lib tho? That would be useful, silica seems to provide customised blocks for weather apps including icons too, but it's hard to find more on that.

@Olf0
Copy link
Member

Olf0 commented Nov 28, 2024

I am struggling with the second line margins, slow process on that, I will send you my patch once it looks ok.

Take your time, it has been like this for a decade. Hence, even if you pause and finish this next year, it will have been a quick action. 😃

Is three anyway to find out whats in silica.weather lib tho? That would be useful, silica seems to provide customised blocks for weather apps including icons too, but it's hard to find more on that.

Foreca Weather is proprietary software from Foreca, licensed by Jolla (as Jolla's Weather app), and IIRC Silica is proprietary software from Jolla. But if any of the parts relevant for your task are written in QML, we can take a look at them.
I have some phones with old SailfishOS releases ([email protected], [email protected]) around, if you can provide me with paths where to look, I can share the relevant code lines in a Gist at GitHub this weekend.

@Olf0
Copy link
Member

Olf0 commented Nov 28, 2024

BTW, I found some interesting documentation for Silica in the WWW, but on first sight nothing addresses these page-layout issues.

But when searching for them you can find many postings on e.g. stackexchange.com etc. where people discuss their issues in achieving a specific page-layout in QML.

@pagism
Copy link

pagism commented Nov 29, 2024

Thanks, I've seen some of the links, I will try to centre the second line and then start thinking about more improvements. With current format I do not think it can accommodate more than 4 days on the second line. On C2 there is horizontal space for 5 days.

The original Weather app has more optimisations, like updates on screen unlock, or visiting the events page, etc.

@Olf0
Copy link
Member

Olf0 commented Nov 30, 2024

I will try to centre the second line

Please take a break when you have achieved that, and submit your achievements by a PR, because (as you stated to already have achieved a better layout of the first line) that should fully rectify the distorted looking, current layout.

and then start thinking about more improvements.

Sure, one can always pose another PR. Though it might make sense to discuss specific changes here to obtain a second, third etc., opinion, before implementing them.

Actually I would appreciate much, when you accompany your PR with a screenshot how it looks on your device (naming device and OS release) in a second comment for that PR (mind that the first comment of a PR will be used as commit details message when the PR is being merged).

With current format I do not think it can accommodate more than 4 days on the second line. On C2 there is horizontal space for 5 days.

I am not sure about this impression from the screenshots above: While it looks so on first sight, there may be a few pixels less space than required for an additional icon (fifth / ninth) while maintaining the current horizontal margins to the left and right. When looking at my Xperia X screenshots, I am quite sure that there is no space for a fifth icon in portrait orientation, but it looks like there is plenty of space for an ninth icon in landscape orientation; but I have no idea how it is rendered on other devices.

Hence, as stated: Things should not (I am actually inclined to state "must not") be optimised for a specific device or screen resolution. Mind that MeeCast still fully supports a [email protected], even older SailfishOS releases, AuroraOS and all devices these two OSes run on, including community ports (many of which are stuck on older SailfishOS releaseas, as the Jolla 1 is): I will strongly oppose anything which may break this feat, and I am deliberately using "may", because nobody can possibly test all these combinations. OTOH, all use the long outdated Qt 5.6, so this is a common denominator.

Also as already stated: Please let Qt take care of correct rendering, do not try to override it (just as with classic, static, CSS-less HTML pages). Any hyper-optimisation will likely fail grossly on some device or some SailfishOS / AuroraOS release (including future ones). Thus hyper-optimisations almost always result in a large maintenance burden, due to the corresponding code becoming complex and fragile.

The original Weather app has more optimisations, like updates on screen unlock, or visiting the events page, etc.

Oh, that is completely new territory: Nice to have, but implementing this might be hard, tedious and frustrating.


P.S.: While in QML one usually anchors elements further down in a column on (width, position etc.) of a element further up in a column or on an element one level up, I remember that one can reverse that (but it is tricky in syntax, semantics and limitations), i.e. anchor a higher (level) element on what QML/Qt has chosen for a lower element. E.g. one should be able to render the icon row as centred and then anchor other elements on the left or right border Qt has chosen for the icon row; here it might make sense to anchor the "weather now" icon in the first row to the left border of the icon row and the "Last updated:"-line to the right border of it.

@pagism
Copy link

pagism commented Nov 30, 2024

meecast

Here is a preview of my changes, horizontal spaces seem to be fixed now, I managed to fit 7 days on C2 but on xperias that should be reduced to 5 days.

I tried to resemble the Jolla weather display, but mainly it's my preferable weather view, it can be adapt it if necessary, I do not have strong opinions on that.

There are several things that need to be fixed:

  • the top line should display hourly temperature, which I do not know how to get it from the app
  • the number of days fit in the 2nd line should be adjusted automatically, the key element here is to work out how many days will fit in the row.
  • the row elements for days are statically coded, that should be converted to a ListView like
  • the update mechanism needs revisiting, the Jolla weather QML code seems to update on several events including turn on the screen, etc.
  • ideally would be nice to include a graph for day temperatures (optional)
  • the icon set does not have the same height for all icons, personally i would prefer to use the existing silica weather icon set, but that might require changes to the actual app code.

PS I tried to create a branch for this issue but I am not sure I have enough permissions to do so.

@Olf0
Copy link
Member

Olf0 commented Dec 1, 2024

[…]

Here is a preview of my changes, horizontal spaces seem to be fixed now,

This is looking really good, thank you! But I see that you already went far beyond fixing the (formerly off) horizontal alignment(s). Though I am repeating myself: Please do let us finish this first, without addressing any other improvements.

Things I notice:

  • The textual description of the current weather is gone. Why?
  • There is a kind of up-arrow there. It appears to be hiding rsp. exposing the icon row; if so, please adhere to the KISS-principle and eliminate it, I can already imagine issue reports stating "MeeCast Eventview does not display any forecast, just the current weather".
  • The whole column (by MeeCast Eventview) still seems to be shifted to he left (i.e. this aspect looks unaltered), i.e. the right margin is smaller than the left margin (between rightmost rsp. leftmost elements and the corresponding screen border). It would be nice to rectify that, if you can determine the reason for it.

I managed to fit 7 days on C2 but on xperias that should be reduced to 5 days.

Be assured I will also test on a Jolla 1, just to take as many devices into account as possible. We will see, how many icons can be practically used on all devices, I would assume 6 in portrait an 12 in landscape orientation.

I tried to resemble the Jolla weather display, but mainly it's my preferable weather view, it can be adapt it if necessary, I do not have strong opinions on that.

Me neither, but I do prefer basic layout of the icons MeeCast currently has, because that is what I looked at every day for more than 10 years. But as rearranging the temperature values was crucial to display more icons in the forecast row, this is fine; as this breaks the original style, mimicking the style of Jolla / Foreca Weather makes sense.

There are several things that need to be fixed:

No, definitely not in your upcoming PR: Please don't, defer researching and implementing these points to later and hence another PR.

  • the top line should display hourly temperature, which I do not know how to get it from the app
  • the number of days fit in the 2nd line should be adjusted automatically, the key element here is to work out how many days will fit in the row.
  • the row elements for days are statically coded, that should be converted to a ListView like

While the three points above should be addressed in later PR(s), IMO the next one should not:

  • the update mechanism needs revisiting, the Jolla weather QML code seems to update on several events including turn on the screen, etc.

IMO that is superfluous or even bad:

  • When turning the device from portrait to landscape orientation and back, MeeCast Eventview already switches between (currently) 4 and 8 icons in the row dynamically.
  • One sets MeeCast to update weather data in a specific frequency (e.g. every hour), no automatism should override this, because it may incur costs (e.g. extremely expensive data tariffs when roaming) which the user is not aware of (until the bill comes).
    One can already manually refresh the weather information in MeeCast proper.
    The only thing I am missing is a "WiFi only" option in SettingsUpdate. But that is completely out of scope for your upcoming PR and the aforementioned three additional points, i.e. it should be addressed in yet another, separate PR.
* ideally would be nice to include a graph for day temperatures (optional)

Another completely different topic, another separate PR when you get to that. May I suggest to implement this as a separate element, which also becomes embedded in MeeCast proper.

* the icon set does not have the same height for all icons, personally i would prefer to use the existing silica weather icon set, but that might require changes to the actual app code.

No way:

  • There are many icon sets one can choose from in MeeCast, please notice that everyone who has uploaded screenshots here preferred a different one. If one of these icon set has flaws, it should be fixed, but nothing else than that.
  • Do try to stay away from Jolla's proprietary stuff (e.g. Silica), they love to introduce arbitrary, often breaking changes without notice (they never tell anything, hence every SailfishOS release bears bad surprises for developers). I would assume this icon set to be eliminated sooner or later, "because nothing uses it any more" (Jolla consistently ignores all apps they do not maintain themselves, hence letting MeeCast using it will not change their attitude).

PS I tried to create a branch for this issue but I am not sure I have enough permissions to do so.

You can always create a branch in your clone of this repository (either at GitHub or locally) and pose a PR to origin:master from that.

@pagism
Copy link

pagism commented Dec 1, 2024

That was a preview, it is not finalised I am not a UI expert, and to be honest I've changed my opinion on some changes. I agree there are several issues that might be better to be discussed separately.

What I will try to do initially is to create a branch and commit changes to the current (as the latest release) layout that fix horizontal spaces. Then from that point on we can discuss further improvements.

To answer briefly your points (that reflects my view and therefore it's subjective) the current eventview seems overloaded, it will be better to be minimalistic or it will be confusing and ignored by users.

  • Currently the information on the first line has an icon, day min/max, location, and a short description. Day min/max is also repeat it on the second line, instead it should display the current hour temp prediction. The short text description repeats the same information the icon tries to convey.
  • The arrow up can go.
  • It's difficult to say the eventview is shifted, we can review the margin calculations, so it should be fixable easily.
  • The eventview changes should be equally supported by older devices too.
  • The number of displayed days on the 2nd line should be calculated dynamically (at the moment is static 4 and 8 for horizontal mode), in that way it will display as many days will fit nicely in the horizontal space, it will also eliminate the need to distinguish screen orientation modes.
  • A ListView handling of days will improve the code and make alignments calculations more consistent.
  • I am not going to touch the update eventview mechanism.
  • The visible part of icons does not have the same or similar height on all icons, that can show as a noticeable gap if we decide to put day min temp under the icon.

@pagism
Copy link

pagism commented Dec 1, 2024

I've branched off sailfishos and trying to push to origin but I do not have enough permissions

@Olf0
Copy link
Member

Olf0 commented Dec 1, 2024

That was a preview, it is not finalised

I know. And I am glad you shared it, so you can gather some feedback.

I am not a UI expert,

None of us really is (though I have reviewed many UI changes in the past 20 years privately and at work); but then, in a way we are because we care.

and to be honest I've changed my opinion on some changes. I agree there are several issues that might be better to be discussed separately.

👍

What I will try to do initially is to create a branch and commit changes to the current (as the latest release) layout that fix horizontal spaces. Then from that point on we can discuss further improvements.

👍

To answer briefly your points (that reflects my view and therefore it's subjective) the current eventview seems overloaded, it will be better to be minimalistic or it will be confusing and ignored by users.

  • Currently the information on the first line has an icon, day min/max, location, and a short description. Day min/max is also repeat it on the second line, instead it should display the current hour temp prediction.

Yes, that would be nice, but as that likely requires some more intrusive changes (outside of MeeCast Eventview) to be able to access this data, I strongly prefer that in a separate PR and to consult @vasvlad specifically for that; ultimately the whole MeeCast code is principally his baby.

The short text description repeats the same information the icon tries to convey.

True, but some of the icon sets are ambiguous to interpret, hence I always perceived this textual information as helpful; even if it is just to learn what a specific icon is supposed to express. There is plenty of space for it in the first line, so why not?

  • The arrow up can go.

Thanks. (Still I am a bit curious if my interpretation what it is good for was correct.)

  • It's difficult to say the eventview is shifted, we can review the margin calculations, so it should be fixable easily.

To review that QML code (before deciding if there is a need to change it and how) was exactly my idea. Unfortunately I am out of time for the next 1,5 weeks, but closer to Christmas I am sure willing to support you with my little QML experience.

  • The eventview changes should be equally supported by older devices too.

👍 … and should not impose additional hurdles to adapt Eventview to other Linux distributions, rsp. desktop environments (e.g. KDE, LXQt). AFAIK @vasvlad is persuing this goal in the long run.

  • The number of displayed days on the 2nd line should be calculated dynamically (at the moment is static 4 and 8 for horizontal mode), in that way it will display as many days will fit nicely in the horizontal space, it will also eliminate the need to distinguish screen orientation modes.
  • A ListView handling of days will improve the code and make alignments calculations more consistent.

Fully agreed that both points would constitute a great achievement and improvement. But they alter the Eventview's QML code fundamentally, hence need thorough review and testing, and would overload this PR as well as preventing a significantly enhanced Eventview layout to be deployed to users soon.

  • I am not going to touch the update eventview mechanism.

Sorry, I did nor mean to shrug this idea off. Maybe it even may fit well to accompany the changes required to obtain the hourly weather data. My main point is that the implications of accessing the internet have to be really well considered; in addition to aforementioned point of costs, also: What happens if there is no internet available in this moment?

  • The visible part of icons does not have the same or similar height on all icons, that can show as a noticeable gap if we decide to put day min temp under the icon.

Yes, and it did not really matter before your layout change, likely this is why nobody noticed it before. But as your layout change makes a lot of sense, this is something which should (but not "must") be resolved for your upcoming PR. Things I can think of:

  • Do all icon sets available in MeeCast have varying icon heights? If it is just a few, it matters less.
  • A (quick & dirty) workaround would be to set this to the maximum spacing of all icons in all icon sets in QML, but then the gap would always be there. Still this appears to be much easier and quicker than adjusting the icon(s / icon sets) proper. Again, I hope to be able to support you doing that much closer to Christmas, so maybe a two-stepped approach makes sense: A quick & dirty workaround now, going through the icon sets and fix the ones with varying icon heights later.
  • Edit: Your screenshot is absolutely fine, the lower temperatures are all vertically aligned. Yes, I see a "gap" below the weather icon for "Today", but "it is what it is", i.e. what is wrong with that? I have the felling I still do not fully comprehend the issue you perceive here.

@Olf0
Copy link
Member

Olf0 commented Dec 1, 2024

I've branched off sailfishos and trying to push to origin but I do not have enough permissions

Please "fork" (i.e. clone) this repository in GitHubs web-frontend, e.g. by making a change to something in the sailfishos branch (or manually "fork" this branch into an own repo) and pose a PR to origin:sailfishos from that (again, GitHub's web-frontend is best suited for that).

When you work on a local clone of this repo you should push your changes to a feature-branch in your own GitHub "fork" of MeeCasts source code repository, and then pose a PR from that to origin:sailfishos by GitHub's web-frontend. Subsequent pushes to your feature-branch then will update the PR automatically, until that PR is closed.

@Olf0 Olf0 self-assigned this Dec 1, 2024
pagism added a commit to pagism/meecast that referenced this issue Dec 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants