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

docs: Minor updates to steps 3 & 4 in Klondike tutorial #2626

Merged
merged 4 commits into from
Jul 29, 2023

Conversation

gilescm
Copy link
Contributor

@gilescm gilescm commented Jul 29, 2023

Description

I have just finished steps 1-4 of the Klondike tutorial and noticed a couple of minor things that look to be outdated with the current dart / flame versions.

Step 3 - late keyword isn't required

When following this step the analyser hightlight this rule unnecessary_late (introduced in Dart 2.16.0) which states:

Top-level and static variables with initializers are already evaluated lazily as if they are marked late.

I updated the notes around why the laziness is important so the reader can still understand the reasoning.

Step 4 - local _isDragging vs DragCallbacks.isDragged

Looking through the DragCallbacks mixin I found that onDragStart() and onDragEnd() both set a private _isDragged flag that acts the same as the _isDragging local flag created in the tutorial. This was added in #2472 along with @mustCallSuper annotations on both methods.

So this change brings the docs up to date by having the new onDragStart/onDragEnd methods reference their super's super.onDragStart and super.onDragEnd instead of the local flag, and now onDragUpdate, onDragEnd use if (!isDragged) { .. } instead of if (!_isDragging) { .. }.

Thanks for awesome tutorial! It's really well written and explains everything just as you need it without being overwhelming.

Checklist

  • I have followed the Contributor Guide when preparing my PR.
  • [-] I have updated/added tests for ALL new/updated/fixed functionality.
  • I have updated/added relevant documentation in docs and added dartdoc comments with ///.
  • [-] I have updated/added relevant examples in examples or docs.

Breaking Change?

  • Yes, this PR is a breaking change.
  • No, this PR is not a breaking change.

Also update the first note around lazy loading so the reader still knows why its important to load the variables in a lazy way.
The DragCallbacks mixin contains its own version of the _isDragging flag called _isDragged (accessible via isDragged getter) so we don't need to keep a local flag to check for valid drags
@spydon spydon changed the title docs: minor updates to steps 3 & 4 in Klondike tutorial docs: Minor updates to steps 3 & 4 in Klondike tutorial Jul 29, 2023
Copy link
Member

@spydon spydon left a comment

Choose a reason for hiding this comment

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

Just some trailing spaces that needs to be removed, otherwise it looks good to me.
Thanks for your contribution!

doc/tutorials/klondike/step3.md:58:76 MD009/no-trailing-spaces Trailing spaces [Expected: 0 or 2; Actual: 1]
doc/tutorials/klondike/step3.md:59:76 MD009/no-trailing-spaces Trailing spaces [Expected: 0 or 2; Actual: 1]
doc/tutorials/klondike/step3.md:60:75 MD009/no-trailing-spaces Trailing spaces [Expected: 0 or 2; Actual: 1]
doc/tutorials/klondike/step3.md:61:76 MD009/no-trailing-spaces Trailing spaces [Expected: 0 or 2; Actual: 1]
doc/tutorials/klondike/step4.md:585:97 MD009/no-trailing-spaces Trailing spaces [Expected: 0 or 2; Actual: 1]
doc/tutorials/klondike/step4.md:586:94 MD009/no-trailing-spaces Trailing spaces [Expected: 0 or 2; Actual: 1]

@spydon spydon enabled auto-merge (squash) July 29, 2023 13:05
@gilescm
Copy link
Contributor Author

gilescm commented Jul 29, 2023

@spydon Ah you beat me to it! I hadn't set up markdown linting on my fork yet sorry about that I'm pretty new to open source contributing, should be fixed now.

Thanks for the fast review 🙏

@spydon spydon merged commit 970babe into flame-engine:main Jul 29, 2023
6 checks passed
@gilescm gilescm deleted the docs/update-klondike-tutorial branch July 29, 2023 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants