-
Notifications
You must be signed in to change notification settings - Fork 194
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
Percents + visualization #139
Conversation
I would put this style on the 'bar' element to make it look a little spacier/better:
Maybe the width should also be based on the maximum amount of sessions for an element: And then maybe add those bars for country and operating system tables |
Seems like I checked out an outdated version where not all tables had the bars yet, my bad |
I do think a width relative to the element with most sessions would be nice, so the differences in bar lenghts are more clear at first glance |
style="width: {{location.count|percent:stats.hit_count}}; top: 8px; left: 8px; background-color: var(--color-urge-200-fallback)" | ||
> | ||
</div> | ||
<div class="absolute" style="top: 8px; left: 8px"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
replace class="absolute" style="top: 8px; left: 8px"
with class="relative"
for same effect
Same thing for the other tables
I made my suggested changes in my repo on the percent-visualization-changes branch if you want to use them. About the relative maximum: Plausible also visualizes the top element as a bar with width 100%, what do you think @milesmcc ? |
Yeah I do think it would be good to scale the bars so that the first/highest element has full width. Makes it easier to distinguish the relative sizes. |
I think showing actual percent feels more natural. It's easier to understand and shows more information. |
style="width: {{browser.count|percent:stats.session_count}}; top: 6px; left: 6px; height: calc(100% - 12px); background-color: var(--color-urge-200-fallback)" | ||
> | ||
</div> | ||
<div class="relative"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add back flex items-center
classes to correctly align icon and text better
style="width: {{os.count|percent:stats.session_count}}; top: 6px; left: 6px; height: calc(100% - 12px); background-color: var(--color-urge-200-fallback)" | ||
> | ||
</div> | ||
<div class="relative"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add back flex items-center
classes to correctly align icon and text better
<td class="truncate w-full max-w-0 relative"> | ||
<div | ||
class="absolute h-6" | ||
style="width: {{location.count|percent:stats.hit_count}}; top: 6px; left: 6px; height: calc(100% - 12px); background-color: var(--color-urge-200-fallback)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe move style attributes to css file to minimize duplicate code like in this commit
<td class="truncate w-full max-w-0 relative"> | ||
<div | ||
class="absolute h-6" | ||
style="width: {{location.count|percent:stats.hit_count}}; top: 6px; left: 6px; height: calc(100% - 12px); background-color: var(--color-urge-200-fallback)" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
left: 0;
to align bar with the table header/body seperator line and make it not get cut off by the td
tag when bar is full or close to full width
I do think, for example in this case, that making the bars as big as possible would improve the clarity of the differences between bar widths a lot. I understand that you would then no longer be able to compare the bars between tables, but that is not something I do anyway. Can't speak for others though. I just think there is a lot of unused whitespace that could be used to make the differences between the bars clearer. |
We just want to achieve different things with this visualization. Maybe it should be customizable in settings? |
That sounds good to me :) |
TEMPLATE.env
Outdated
@@ -96,3 +96,6 @@ LOCATION_URL=https://www.openstreetmap.org/?mlat=$LATITUDE&mlon=$LONGITUDE | |||
# How many services should be displayed on dashboard page? | |||
# Set to big number if you don't want pagination at all. | |||
DASHBOARD_PAGE_SIZE=5 | |||
|
|||
# Should background bars be scaled to full width? | |||
USE_RELATIVE_MAX_IN_BAR_VISUALIZATION = True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code style, remove spaces around =
shynet/shynet/settings.py
Outdated
|
||
# How many services should be displayed on dashboard page? | ||
DASHBOARD_PAGE_SIZE = int(os.getenv("DASHBOARD_PAGE_SIZE", "5")) | ||
|
||
# Should background bars be scaled to full width? | ||
USE_RELATIVE_MAX_IN_BAR_VISUALIZATION = True |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
USE_RELATIVE_MAX_IN_BAR_VISUALIZATION = os.getenv("USE_RELATIVE_MAX_IN_BAR_VISUALIZATION", "True") == "True"
style="width: {{count|percent:total}}; top: 6px; left: 0px; height: calc(100% - 12px); background-color: var(--color-urge-200-fallback)" | ||
> | ||
</div> | ||
{% endif %} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
simplify to
<div
class="absolute h-6"
style="width: {% if use_relative_max %}{{count|percent:max}}{% else %}{{count|percent:total}}{% endif %}; top: 6px; left: 0px; height: calc(100% - 12px); background-color: var(--color-urge-200-fallback)"
></div>
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know it probably should be done this way. This line just looks so ugly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know :p
It's ugly code vs duplicate code
</div> | ||
<div class="relative"> | ||
{% include 'dashboard/includes/bar.html' with count=country.count max=stats.countries.0.count total=stats.session_count %} | ||
<div class="relative flex items-center"> | ||
<span class="{{country.country|flag_class}}"></span> {{country.country|country_name}} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add flex-none
class here to make flag not get squished
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
flex-none
needs to be on the
<span class="{{country.country|flag_class}}"></span> {{country.country|country_name}}
line, not the one above
This is fantastic. Merged. Will be in the next release! :) |
I managed to do visualization similar to what is in Plausible.
I'm not very happy about style attributes. I could use some help with that.