Skip to content

Border.Shadow hide the collectionView Header/footer -UITest #25960

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
Dec 12, 2024
Merged

Border.Shadow hide the collectionView Header/footer -UITest #25960

merged 2 commits into from
Dec 12, 2024

Conversation

kubaflo
Copy link
Contributor

@kubaflo kubaflo commented Nov 19, 2024 •
edited
Loading

Description of Change

Whenever a view, wrapped inside a wrapper view, has been a direct child of a header or footer element, it either didn't show at all or rendered incorrectly.

Issues Fixed

Fixes #19338

Before After

@kubaflo kubaflo requested a review from a team as a code owner November 19, 2024 19:45
@kubaflo kubaflo requested review from Eilon and tj-devel709 November 19, 2024 19:45
@dotnet-policy-service dotnet-policy-service bot added the community ✨ Community Contribution label Nov 19, 2024
@rmarinho
Copy link
Member

/rebase

@rmarinho
Copy link
Member

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

jsuarezruiz
jsuarezruiz requested changes Nov 20, 2024
View reviewed changes
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue19338.cs
public void Issue935TestsMultipleOnTappedViewCell()
{
App.WaitForElement("HeaderLabel");
VerifyScreenshot();
Copy link
Contributor

Choose a reason for hiding this comment

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

Snapshots available in the latest build:
image
Could you commit the images?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had to add margin to the collection, so I need to re-upload screenshots
HeaderAndFooterShouldBeVisible

@rmarinho
Copy link
Member

/rebase

@rmarinho
Copy link
Member

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@rmarinho
Copy link
Member

/azp run maui-pr

@azure-pipelines Azure Pipelines
Copy link

No pipelines are associated with this pull request.

@jsuarezruiz jsuarezruiz added area-controls-collectionview CollectionView, CarouselView, IndicatorView area-controls-border Border labels Nov 21, 2024
jsuarezruiz
jsuarezruiz requested changes Nov 21, 2024
View reviewed changes
Copy link
Contributor

@jsuarezruiz jsuarezruiz left a comment •
edited
Loading

Choose a reason for hiding this comment

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

The test HeaderAndFooterShouldBeVisible is failing on all the platforms. After the margin change, need to update the snapshot in all the platforms. Images available in the latest build.

@kubaflo kubaflo requested a review from jsuarezruiz November 21, 2024 10:31
@jsuarezruiz
Copy link
Contributor

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@rmarinho
Copy link
Member

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

rmarinho
rmarinho reviewed Nov 27, 2024
View reviewed changes
Copy link
Member

@rmarinho rmarinho left a comment

Choose a reason for hiding this comment

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

Does this work fine with CollectionView2 ?

@kubaflo
Copy link
Contributor Author

kubaflo commented Nov 27, 2024

Hi, @rmarinho I was testing it on the sandbox project on the latest main which I think uses cv2, right?

@rmarinho
Copy link
Member

It depends the time you tested.. but this fix is for CV1

@kubaflo
Copy link
Contributor Author

kubaflo commented Nov 27, 2024 •
edited
Loading

I tried on today's main and this is the result for cv1:

And this one is for the cv2

So it appears the problem is only with the cv1

@rmarinho should I close this PR then?

@PureWeen PureWeen modified the milestones: .NET 9 SR2, .NET 9 SR3 Dec 6, 2024
@PureWeen
Copy link
Member

PureWeen commented Dec 7, 2024

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@rmarinho rmarinho requested review from rmarinho and PureWeen December 9, 2024 11:54
rmarinho
rmarinho requested changes Dec 11, 2024
View reviewed changes
Copy link
Member

@rmarinho rmarinho left a comment

Choose a reason for hiding this comment

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

Can we rebase and see how it works with the new container?

@kubaflo kubaflo requested a review from jfversluis as a code owner December 11, 2024 10:29
@kubaflo
Copy link
Contributor Author

kubaflo commented Dec 11, 2024

@rmarinho yess it works with the new generalWrapperView! But, maybe let's keep the UI test?

@kubaflo kubaflo changed the title Border.Shadow hide the collectionView Header/footer - fix Border.Shadow hide the collectionView Header/footer -UITest Dec 11, 2024
@rmarinho
Copy link
Member

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

rmarinho
rmarinho requested changes Dec 11, 2024
View reviewed changes
Copy link
Member

@rmarinho rmarinho left a comment

Choose a reason for hiding this comment

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

Do we need new images ?

CV1
image

CV2
image

@rmarinho
Copy link
Member

Diff
image

@kubaflo
Copy link
Contributor Author

kubaflo commented Dec 11, 2024

@rmarinho yea I modified the test a bit and was waiting to re-upload snapshots. It should be green now :)

@rmarinho
Copy link
Member

/azp run

@azure-pipelines Azure Pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@kubaflo
Copy link
Contributor Author

kubaflo commented Dec 12, 2024

@rmarinho I think the tests passed for this issue

@rmarinho rmarinho merged commit 612b4b1 into dotnet:main Dec 12, 2024
104 checks passed
@samhouts samhouts added fixed-in-net9.0-nightly This may be available in a nightly release! fixed-in-net8.0-nightly This may be available in a nightly release! labels Dec 16, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Jan 16, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@rmarinho rmarinho rmarinho approved these changes

@jsuarezruiz jsuarezruiz Awaiting requested review from jsuarezruiz

@PureWeen PureWeen Awaiting requested review from PureWeen

@jfversluis jfversluis Awaiting requested review from jfversluis

Assignees

@rmarinho rmarinho

Labels
area-controls-border Border area-controls-collectionview CollectionView, CarouselView, IndicatorView community ✨ Community Contribution fixed-in-net8.0-nightly This may be available in a nightly release! fixed-in-net9.0-nightly This may be available in a nightly release!
Projects
MAUI SDK Ongoing
Status: Done
Milestone
.NET 9 SR3
Development

Successfully merging this pull request may close these issues.

[iOS] Border.Shadow hide the collectionView Header
5 participants
@kubaflo @rmarinho @jsuarezruiz @PureWeen @samhouts