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

feat(History): add option to show stats in different values #2007

Merged
merged 12 commits into from
Nov 9, 2024

Conversation

meteyou
Copy link
Member

@meteyou meteyou commented Sep 11, 2024

Description

This PR adds an option to switch the stats (circle or table) in the history page between amount, filament and time.

Related Tickets & Documents

fixes #1966

Mobile & Desktop Screenshots/Recordings

Circle Amount:
image

Circle Filament:
image

Circle Time:
image

Table Amount:
image

Table Filament:
image

Table Time:
image

[optional] Are there any post-deployment tasks we need to perform?

none

Summary by Sourcery

Introduce a feature to toggle the display of history statistics between different units (amount, filament, time) and refactor the code to improve maintainability by using a mixin for history statistics logic.

New Features:

  • Add an option to switch the display of statistics on the history page between amount, filament, and time.

Enhancements:

  • Refactor the history statistics logic into a new mixin to streamline the handling of different statistic types.

Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Sep 11, 2024
Copy link
Contributor

Language file analysis report:

File Missing Keys Unused Keys
en.json 0 0

@meteyou
Copy link
Member Author

meteyou commented Sep 15, 2024

@sourcery-ai review

Copy link
Contributor

sourcery-ai bot commented Sep 15, 2024

Reviewer's Guide by Sourcery

This pull request adds a new feature to the history page, allowing users to switch between different statistical views (amount, filament, time) for both the circle chart and table display. The implementation involves refactoring existing components, introducing a new mixin for shared logic, and updating the UI to include toggle buttons for the new options.

File-Level Changes

Change Details Files
Introduced a new HistoryStatsMixin to handle shared logic for history statistics
  • Created a new file 'src/components/mixins/historyStats.ts'
  • Implemented methods to process and format chart data
  • Added logic to group small entries for better data representation
src/components/mixins/historyStats.ts
Updated HistoryAllPrintStatusChart component to support multiple value types
  • Added a new prop 'valueName' to switch between amount, filament, and time
  • Updated chart options to format tooltip values based on the selected value type
  • Refactored the component to use the new HistoryStatsMixin
src/components/charts/HistoryAllPrintStatusChart.vue
Refactored HistoryAllPrintStatusTable component
  • Created a new HistoryAllPrintStatusTableItem component for individual table rows
  • Updated the table to use the new item component and support multiple value types
  • Integrated the HistoryStatsMixin for shared logic
src/components/charts/HistoryAllPrintStatusTable.vue
src/components/charts/HistoryAllPrintStatusTableItem.vue
Updated HistoryStatisticsPanel to include new toggle options
  • Added a new button group to switch between amount, filament, and time views
  • Updated child components to receive the selected value type
src/components/panels/HistoryStatisticsPanel.vue
Modified server history state types and getters
  • Added new fields 'valueFilament' and 'valueTime' to ServerHistoryStateAllPrintStatusEntry
  • Created a new type 'HistoryStatsValueNames' for the available value options
  • Removed redundant getters from the history store
src/store/server/history/types.ts
src/store/server/history/getters.ts
Updated localization files
  • Added new translations for 'Amount', 'Filament', and 'Time' in the History section
src/locales/en.json

Tips
  • Trigger a new Sourcery review by commenting @sourcery-ai review on the pull request.
  • Continue your discussion with Sourcery by replying directly to review comments.
  • You can change your review settings at any time by accessing your dashboard:
    • Enable or disable the Sourcery-generated pull request summary or reviewer's guide;
    • Change the review language;
  • You can always contact us if you have any questions or feedback.

sourcery-ai[bot]
sourcery-ai bot previously approved these changes Sep 15, 2024
Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @meteyou - I've reviewed your changes and they look great!

Here's what I looked at during the review
  • 🟡 General issues: 2 issues found
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟡 Complexity: 1 issue found
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

src/components/charts/HistoryAllPrintStatusChart.vue Outdated Show resolved Hide resolved
src/components/mixins/historyStats.ts Show resolved Hide resolved
Signed-off-by: Stefan Dej <meteyou@gmail.com>
Signed-off-by: Stefan Dej <meteyou@gmail.com>
@meteyou
Copy link
Member Author

meteyou commented Sep 15, 2024

@sourcery-ai review

Copy link
Contributor

Language file analysis report:

File Missing Keys Unused Keys
en.json 0 0

sourcery-ai[bot]
sourcery-ai bot previously approved these changes Sep 15, 2024
Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @meteyou - I've reviewed your changes and they look great!

Here's what I looked at during the review
  • 🟡 General issues: 1 issue found
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟡 Complexity: 1 issue found
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

src/components/mixins/historyStats.ts Show resolved Hide resolved
src/components/mixins/historyStats.ts Outdated Show resolved Hide resolved
Signed-off-by: Stefan Dej <meteyou@gmail.com>
Copy link
Contributor

Language file analysis report:

File Missing Keys Unused Keys
en.json 0 0

@meteyou
Copy link
Member Author

meteyou commented Sep 15, 2024

@sourcery-ai review

sourcery-ai[bot]
sourcery-ai bot previously approved these changes Sep 15, 2024
Copy link
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey @meteyou - I've reviewed your changes and they look great!

Here's what I looked at during the review
  • 🟡 General issues: 1 issue found
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟡 Complexity: 1 issue found
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.

src/locales/en.json Outdated Show resolved Hide resolved
Signed-off-by: Stefan Dej <meteyou@gmail.com>
Copy link
Contributor

Language file analysis report:

File Missing Keys Unused Keys
en.json 0 0

Signed-off-by: Stefan Dej <meteyou@gmail.com>
Copy link
Contributor

Language file analysis report:

File Missing Keys Unused Keys
en.json 0 0

Copy link
Contributor

Language file analysis report:

File Missing Keys Unused Keys
en.json 0 0

@rackrick
Copy link
Member

rackrick commented Nov 4, 2024

@meteyou there still two pending comments. I would at least adjust the rounding before merging.

…ableItem

Signed-off-by: Stefan Dej <meteyou@gmail.com>
Copy link
Contributor

github-actions bot commented Nov 5, 2024

Language file analysis report:

File Missing Keys Unused Keys
en.json 0 0

Signed-off-by: Stefan Dej <meteyou@gmail.com>
Copy link
Contributor

github-actions bot commented Nov 5, 2024

Language file analysis report:

File Missing Keys Unused Keys
en.json 0 0

Signed-off-by: Stefan Dej <meteyou@gmail.com>
Copy link
Contributor

github-actions bot commented Nov 7, 2024

Language file analysis report:

File Missing Keys Unused Keys
en.json 0 0

@dosubot dosubot bot added the lgtm This PR has been approved by a maintainer label Nov 8, 2024
@meteyou meteyou merged commit fbeecb0 into mainsail-crew:develop Nov 9, 2024
12 checks passed
@meteyou meteyou deleted the feat/print-status-types branch November 9, 2024 00:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm This PR has been approved by a maintainer size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Option to show history chart/table as time or filament used instead of number of prints
2 participants