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

Fix: StaggeredLayout division bug #574

Merged
merged 1 commit into from
Dec 3, 2024

Conversation

Poker-sang
Copy link
Contributor

Fixes

closes #542
fix microsoft/PowerToys#35139

PR Type

What kind of change does this PR introduce?

What is the current behavior?

What is the new behavior?

PR Checklist

Please check if your PR fulfills the following requirements:

  • Created a feature/dev branch in your fork (vs. submitting directly from a commit on main)
  • Based off latest main branch of toolkit
  • Tested code with current supported SDKs
  • New component
    • Documentation has been added
    • Sample in sample app has been added
    • Analyzers are passing for documentation and samples
    • Icon has been created (if new sample) following the Thumbnail Style Guide and templates
  • Tests for the changes have been added (if applicable)
  • Header has been added to all new source files
  • Contains NO breaking changes

Other information

@michael-hawker
Copy link
Member

Not sure what happened with the tooling submodule here either.

Wondering if we could add a test for anything around this to help guard future issues?

@Poker-sang
Copy link
Contributor Author

Poker-sang commented Nov 28, 2024

Not sure what happened with the tooling submodule here either.

Wondering if we could add a test for anything around this to help guard future issues?

reverted too. Is there a way to make git not track changes to submodules?

@michael-hawker michael-hawker added this to the 8.2 milestone Dec 2, 2024
Copy link
Member

@michael-hawker michael-hawker left a comment

Choose a reason for hiding this comment

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

🦙❤️ Thanks!

Copy link
Member

@michael-hawker michael-hawker left a comment

Choose a reason for hiding this comment

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

Was just about to approve and merge, but noticed it was slightly different than when I had first looked before the weekend. Thanks for fixing the tooling submodule stuff.

@Poker-sang
Copy link
Contributor Author

Poker-sang commented Dec 3, 2024

@michael-hawker Ok reverted it. i commit this ((columnWidth + ColumnSpacing) instead of columnWidth) here because i found that we forget to calculate spacing before. And we might see the bug clearly when ColumnSpacing is close to columnWidth. And you are right we should make it in #573

I moved it here

image

@michael-hawker
Copy link
Member

🎉 Thanks @Poker-sang for this quick fix! 🦙❤️ And for moving the column fix to the other PR. I'll get this merged once the CI passes, some strange issue I haven't seen before there; so just re-kicking to see if it was just a stray cosmic ray! 🤞

@michael-hawker michael-hawker enabled auto-merge (rebase) December 3, 2024 20:17
@michael-hawker michael-hawker merged commit c756dd5 into CommunityToolkit:main Dec 3, 2024
24 checks passed
@Poker-sang Poker-sang deleted the fix-division-bug branch December 4, 2024 00:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

StaggeredLayout crashes on ARM64 build
2 participants