-
Notifications
You must be signed in to change notification settings - Fork 244
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
feat: add grid lines #1179
Open
JulienDeveaux
wants to merge
16
commits into
kalkih:dev
Choose a base branch
from
JulienDeveaux:add-grid-lines
base: dev
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
feat: add grid lines #1179
Changes from 12 commits
Commits
Show all changes
16 commits
Select commit
Hold shift + click to select a range
9dfa15b
feat: add grid lines
JulienDeveaux e875f7c
style: better looking grid lines
JulienDeveaux 70105ef
refactor: move grid_line_type to show options & add 5minute variant
JulienDeveaux 5956678
style: use ha variable color & let user decide thick / thin lines fre…
JulienDeveaux 6e6492b
fix: redundant stroke
JulienDeveaux 9dd491a
update readme
JulienDeveaux cdc7451
feat: safety check on grid lines ratio & update readme
JulienDeveaux d905261
fix readme
JulienDeveaux a950796
style: better grid lines customizability
JulienDeveaux 8d47c1c
style: better classes & doc
JulienDeveaux abbddeb
refactor: render grid lines algo from ildar170975
JulienDeveaux e8355ea
fix: build
JulienDeveaux 09fc231
refactor: simplify grid line strokes
JulienDeveaux c7a0a8b
refactor: move all grid lines to different g tag
JulienDeveaux 9b7c879
add missing class
JulienDeveaux fd1bfe2
fix: class grid-lines to grid--lines
JulienDeveaux File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Let's change it to:
grid_lines_type |
hour
|false
/5minute
/hour
/day
/week
| Show grid lines dependently onhours_to_show
value (set tofalse
or omit this option if grid lines should not be displayed).I hope this code:
does process the
show.grid_lines_type = false
case properly.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.
It should. However, it could be simplified. ;-)
Also, I think, we should consider to have an option of grid_lines_per_hour, rather than grid_lines_type. It would allow for more flexibility with much simpler code. But I haven't thought this to the end yet.
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.
The initial idea was:
-- "main" (thick) lines are shown every 5 minutes / hourly / daily / ... ;
-- "secondary" (thin) lines are shown more frequently ("ratio" option) or not shown at all.
I honestly cannot see how
grid_lines_per_hour
may be used along with some "show secondary lines" option (or I am still far away from HA & normal life, need more time)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.
Just from the top of my head, I was thinking:
points_per_hour
setting. And reduce repeated code to actually convert days or weeks into multiples of hours.)nth-of-type
in css instead. Again it reduces code and is more customizable by the user.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.
Please let me think a bit.
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.
@akloeckner
Was thinking about using
grid_lines_per_hour
:Currently we got new proposed options -
grid_lines_type
(false
/5minute
/hour
/day
/week
) &grid_lines_ratio
(integer, >=0).Alternatively, we could get smth like
grid_lines_per_hour
(for thick lines) and samegrid_lines_ratio
(for thin lines).And in this case this
grid_lines_per_hour
is numerical (could be fractional < 1).Let's consider the "<1" case - it is needed to show thick lines, for example, every 2 hours, every day etc.
For the "every day" case a user will have to use a
0.041667
value.Does not look nice, a
1/24
value seems to be more user-friendly - but in this case we need to process these1/x
values and convert them to valid floats.If we choose the current implementation with
grid_lines_type
- we will have a minimal granularity for thick lines = 5 minutes.A smaller granularity then could be achieved by
grid_lines_ratio
:5 - thin lines every 1 minute,
10 - every 30 seconds, and so on.
Probably this would be sufficient for most scenarios.
Sorry, I think using
nth-of-type
will make thing more difficult.Assume we got a set of thick & thin lines WITHOUT classes.
To style them, we could use
nth-of-type
or similar - but a formula could be complex.We draw these lines from a right side (now -> past). So, we will have to process these
nth
from the end as well. Not easy I think.Using a separate class for thick & thin lines will allow users to quickly show/hide them by card-mod.