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

TimeSeriesChart: Add time label format option for time series chart #9049

Merged

Conversation

jorisBarkema
Copy link
Contributor

@jorisBarkema jorisBarkema commented May 24, 2024

The time label format was hardcoded at HH:mm, but I wanted a chart over several days. This format does not make sense in that case so I wanted a way to show the date in the time label.

Description

Add a parameter similar to TimeLabelSpacing to be able to set the TimeLabelFormat. Default to the value that was previously hardcoded to maintain backwards compatability.

How Has This Been Tested?

The TimeSeries component as a whole has no tests yet (#8973 see last comments) so can't add a test for just this feature without testing the whole thing and I sadly don't have time for that.

Type of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation (fix or improvement to the website or code docs)

Example of a time series with datelabel format "dd/MM HH:mm"
example_timeseries_with_dates

Checklist

  • The PR is submitted to the correct branch (dev).
  • My code follows the code style of this project.
  • I've added relevant tests.

@github-actions github-actions bot added enhancement New feature or request PR: needs review labels May 24, 2024
Copy link

codecov bot commented May 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.62%. Comparing base (28bc599) to head (7a6b0e5).
Report is 232 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev    #9049      +/-   ##
==========================================
+ Coverage   89.82%   90.62%   +0.79%     
==========================================
  Files         412      398      -14     
  Lines       11878    12372     +494     
  Branches     2364     2403      +39     
==========================================
+ Hits        10670    11212     +542     
+ Misses        681      621      -60     
- Partials      527      539      +12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ScarletKuro
Copy link
Member

Hi,

Thanks for the PR. Can you add a very simple bUnit test with a custom TimeLabelFormat to ensure that it renders appropriately? Thanks.

@jorisBarkema jorisBarkema marked this pull request as draft May 24, 2024 11:17
@jorisBarkema jorisBarkema marked this pull request as ready for review May 24, 2024 12:02
@jorisBarkema
Copy link
Contributor Author

Test has been added now

@ScarletKuro
Copy link
Member

Thanks

@henon henon merged commit 2f97327 into MudBlazor:dev May 24, 2024
4 checks passed
@radderz
Copy link
Contributor

radderz commented May 30, 2024

My bad I had intended for it to have a date/time format but I must have missed that with tunnel vision!

@radderz
Copy link
Contributor

radderz commented May 30, 2024

@jorisBarkema I also thought about making the labels be able to have an angle to make it work better in higher density data. I.e. 30-45 degree angle labels.

Otherwise did you find it a good chart?

@jorisBarkema
Copy link
Contributor Author

Yeah it was very easy to work with and did exactly what I wanted to do. So far at least because some other things have come up after I started on this.
I think the setting for the angles is a good idea, and there are probably some more options that can be added, but it's only natural that once a new feature like this is added it slowly starts getting more customization features like that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants