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

Use Selectorbar for Sample Code #1540

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

ghost1372
Copy link
Contributor

related to #1535

Full Code (Xaml/C#)

00002

Single Code (Xaml Only)

00001

Description

Motivation and Context

How Has This Been Tested?

Tested on my Laptop

Screenshots (if appropriate):

Types 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 change)

@Jay-o-Way
Copy link
Contributor

Does this solve #704?

@ghost1372
Copy link
Contributor Author

ghost1372 commented May 18, 2024

Does this solve #704?

No. we need to disable horizontal scroll

Copy link
Collaborator

@marcelwgn marcelwgn left a comment

Choose a reason for hiding this comment

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

Thank you for creating this PR!

I think porting the SwitchPresenter control over just for this is a bit overkill, this problem can also be solved with a bit of code-behind. Also, given that people also use this app's source code for reference, introducing a new control just for this doesn't seem like the best idea.

@ghost1372
Copy link
Contributor Author

Thank you for creating this PR!

I think porting the SwitchPresenter control over just for this is a bit overkill, this problem can also be solved with a bit of code-behind. Also, given that people also use this app's source code for reference, introducing a new control just for this doesn't seem like the best idea.

I saw that the gallery uses some communitytoolkit Nuget packages.
For a simple control, I did not use the community Nuget package, which has a larger size...
That's why I copied their codes.

@marcelwgn
Copy link
Collaborator

We are in fact using Community Toolkit packages, that is correct. I would argue though, in this case, porting a complete control for something one could do with a few lines of code behind is overkill.

@ghost1372
Copy link
Contributor Author

We are in fact using Community Toolkit packages, that is correct. I would argue though, in this case, porting a complete control for something one could do with a few lines of code behind is overkill.

ok, what do you suggest? Changing visibility? Frame?

@marcelwgn
Copy link
Collaborator

ok, what do you suggest? Changing visibility? Frame?

I would probably use a ContentPresenter and have that switch the content

@ghost1372
Copy link
Contributor Author

@marcelwgn I saw your last message late, I sent 2 commits, First by changing the visibility and lastly the suggestion you gave.

@Jay-o-Way

This comment was marked as resolved.

@ghost1372
Copy link
Contributor Author

@Jay-o-Way i reverted last commit (ContentPresenter) now everything is working fine

Jay-o-Way

This comment was marked as outdated.

@Jay-o-Way
Copy link
Contributor

Does this solve #704?

No. we need to disable horizontal scroll

For me, this pr currently actually does disable horizontal movement when using the scroll wheel hovering this area. 🥳 @karkarl

ghost1372 and others added 2 commits May 20, 2024 09:56
Co-authored-by: Jay <65828559+Jay-o-Way@users.noreply.github.com>
@karkarl
Copy link
Collaborator

karkarl commented May 20, 2024

Does this solve #704?

No. we need to disable horizontal scroll

For me, this pr currently actually does disable horizontal movement when using the scroll wheel hovering this area. 🥳 @karkarl

I'm ok disabling horizontal scroll. It annoys me a lot too :P

@Jay-o-Way
Copy link
Contributor

Question: I see muxc:Expander is used, but do we still need the muxc part?
One more detail please? Removing this margin/padding would be pretty.
image

@ghost1372
Copy link
Contributor Author

Does this solve #704?

No. we need to disable horizontal scroll

For me, this pr currently actually does disable horizontal movement when using the scroll wheel hovering this area. 🥳 @karkarl

I'm ok disabling horizontal scroll. It annoys me a lot too :P

done

Jay-o-Way

This comment was marked as resolved.

@ghost1372
Copy link
Contributor Author

Hold on, I see a little misunderstanding here. The issue in 704 was: When scrolling (vertically) across a page, coming over a code example gives an unwanted horizontal scrolling. This PR already solved that. The last commit makes it impossible to scroll horizontally. Please revert.

Done

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.

None yet

5 participants