Tucsky / aggr

Cryptocurrency trades aggregator
https://charts.aggr.trade/
GNU General Public License v3.0
801 stars 232 forks source link

Small UX improvements of the Chart overlay #389

Closed adeacetis closed 6 months ago

adeacetis commented 6 months ago

This Pull Request addresses a comment found in the Discord channel where the user was proposing to make the Add label of the Chart Overlay always visible instead of revealing on the hover of the drop-down button.

https://discord.com/channels/1110160121813803058/1115914099071656050/1189675001473937408

The current implementation creates a small jump in the UI that is discomforting for the user when it only wants to reveal / then hide the list of indicators or markets which is expected to be done with a double click and not 'click-move to right-click'

The PR also improves the readability of the market overlay label by adding spaces around the separator '|'.

Add button always visible

Tucsky commented 6 months ago

i hide it because it's taking too much unecessary space when hovering a chart (on a small chart) maybe the word "indicators" is too long as well, ideally should be an icon 😃

adeacetis commented 6 months ago

i hide it because it's taking too much unecessary space when hovering a chart (on a small chart) maybe the word "indicators" is too long as well, ideally should be an icon 😃

Ideally, it should be part of a toolbar which we don't have, and/or in a menu,

1) If you are hovering over a chart, you are expecting to make an action and therefore accept the compromise comfort for information. The proposed solution, at least, for new users, makes it easier to see where they could add an indicator at first glance. Additionally, the top left location isn't that much of an issue since it barely overlays the mid-section of the chart on a default template.

2) Also if you look at the minimal footprint, when indicator values are NA, the menu titles' width is still smaller than the indicators, so I don't think it matters that much.

image

3) On mobile, adding an indicator, on the current implementation is a three-step action and it could be more frustrating than having the add button

There are also additional issues on mobile such as difficulties in opening the overlay menu because of unexpected double touch if you try to open the indicator menu.

Instead of opening the drop-down, you open the add indicator overlay by magic. (tested on Android) That might be due to the bouncing effect as both the indicator list open button and indicator dialog open button are stacked on the UI. Maybe we should debounce the event listener to have better results. Showing permanently the Add button is a better approach. There is no surprise for the user. However, we should make both targets slightly bigger, it's a bit uncomfortable imo.

Side note: Watch how I am struggling to display the overlay. I haven't looked in the code yet but instinctively I couldn't make it - needed several taps.

https://github.com/Tucsky/aggr/assets/20586054/26873f61-6e2f-4aec-9d40-a8b6d367430f

Tucsky commented 6 months ago

no problem ! was just explaining the why

Tucsky commented 6 months ago

Ideally, it should be part of a toolbar which we don't have, and/or in a menu

we do have image

we even have 2 per charts 😃

adeacetis commented 6 months ago

Does that mean: "LGTM, please rebase and merge"?

Tucsky commented 6 months ago

Does that mean: "LGTM, please rebase and merge"?

Lol yes sorry looks like I don't have any approve button on your PRs 😉