Skip to content

feat: background line thickness settings added #1192

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Feb 22, 2025
Merged

feat: background line thickness settings added #1192

merged 2 commits into from
Feb 22, 2025

Conversation

arpandaze
Copy link
Contributor

Adds settings to control the thickness of background lines.

@arpandaze arpandaze force-pushed the background-line branch 2 times, most recently from ed87823 to d3dbac9 Compare May 15, 2024 16:34
adil192
adil192 requested changes Nov 10, 2024
View reviewed changes
Copy link
Member

@adil192 adil192 left a comment

Choose a reason for hiding this comment

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

This is cool, can you just refactor lineWeight to lineThickness. Atm it looks too similar to lineHeight!

@arpandaze arpandaze force-pushed the background-line branch 2 times, most recently from 834e687 to 4951224 Compare November 13, 2024 15:12
@arpandaze
Copy link
Contributor Author

This is cool, can you just refactor lineWeight to lineThickness. Atm it looks too similar to lineHeight!

Sure. I have just refactored it into lineThickness but pipeline seems to be failing due to late initialization in 1 test. I have also added default value for the preference but not sure why its throwing error. Can you please let me know how to fix this?

@codecov Codecov
Copy link

codecov bot commented Feb 22, 2025

Codecov Report

Attention: Patch coverage is 35.89744% with 25 lines in your changes missing coverage. Please review.

Project coverage is 47.15%. Comparing base (e81fab0) to head (6bf96d7).

Files with missing lines Patch % Lines
lib/components/toolbar/editor_bottom_sheet.dart 0.00% 16 Missing ⚠️
lib/pages/editor/editor.dart 0.00% 5 Missing ⚠️
lib/data/editor/editor_core_info.dart 72.72% 3 Missing ⚠️
...b/components/canvas/canvas_background_preview.dart 0.00% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #1192      +/-   ##
==========================================
- Coverage   47.22%   47.15%   -0.07%     
==========================================
  Files         114      114              
  Lines        8966     9002      +36     
==========================================
+ Hits         4234     4245      +11     
- Misses       4732     4757      +25

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

@adil192
Copy link
Member

adil192 commented Feb 22, 2025

Sure. I have just refactored it into lineThickness but pipeline seems to be failing due to late initialization in 1 test. I have also added default value for the preference but not sure why its throwing error. Can you please let me know how to fix this?

Oof yeah that was a tricky issue.
The reason was that large files (like v6_stress_test_179.sbn) are parsed on a separate isolate which doesn't have access to Prefs.
I fixed it by falling back to the default line thickness instead of the last line thickness if we're not on the main isolate

@adil192 adil192 merged commit 721f1e4 into saber-notes:main Feb 22, 2025
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adil192 adil192 adil192 requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@arpandaze @adil192