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.

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

Fix #4940: scroll block applied incorrectly in some cases #4946

Merged
merged 4 commits into from
Sep 23, 2023
Merged

Fix #4940: scroll block applied incorrectly in some cases #4946

merged 4 commits into from
Sep 23, 2023

Conversation

avasuro
Copy link
Contributor

@avasuro avasuro commented Sep 23, 2023 •
edited
Loading

Fix #4940

@vercel
Copy link

vercel bot commented Sep 23, 2023 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
primereact ⬜️ Ignored (Inspect) Visit Preview Sep 23, 2023 0:45am

@github-actions
Copy link

Thanks a lot for your contribution! But, PR does not seem to be linked to any issues. Please manually link to an issue or mention it in the description using #<issue_id>.

@avasuro avasuro changed the title fix(Dialog): scroll block applied incorrectly in some cases fix #4946: scroll block applied incorrectly in some cases Sep 23, 2023
@github-actions
Copy link

Thanks a lot for your contribution! But, PR does not seem to be linked to any issues. Please manually link to an issue or mention it in the description using #<issue_id>.

@avasuro avasuro changed the title fix #4946: scroll block applied incorrectly in some cases Fix #4946: scroll block applied incorrectly in some cases Sep 23, 2023
@github-actions
Copy link

Thanks a lot for your contribution! But, PR does not seem to be linked to any issues. Please manually link to an issue or mention it in the description using #<issue_id>.

@avasuro avasuro changed the title Fix #4946: scroll block applied incorrectly in some cases Fix #4940: scroll block applied incorrectly in some cases Sep 23, 2023
@github-actions
Copy link

Thanks a lot for your contribution! But, PR does not seem to be linked to any issues. Please manually link to an issue or mention it in the description using #<issue_id>.

1 similar comment
@github-actions
Copy link

Thanks a lot for your contribution! But, PR does not seem to be linked to any issues. Please manually link to an issue or mention it in the description using #<issue_id>.

@melloware melloware added the Type: Enhancement Issue contains an enhancement related to a specific component. Additional functionality has been add label Sep 23, 2023
melloware
melloware approved these changes Sep 23, 2023
View reviewed changes
Copy link
Member

@melloware melloware left a comment

Choose a reason for hiding this comment

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

This looks great. Let me test it out.

melloware
melloware requested changes Sep 23, 2023
View reviewed changes
components/lib/dialog/Dialog.js Outdated Show resolved Hide resolved
@melloware
Copy link
Member

On more thing while you are in there my IDE is showing the method getPositionClass as no longer used and I just removed it and verified all the position samples are working correctly. So its something that got moved with unstyled mode but this code was not deleted. Do you mind deleting it as part of this PR please?

@github-actions
Copy link

Thanks a lot for your contribution! But, PR does not seem to be linked to any issues. Please manually link to an issue or mention it in the description using #<issue_id>.

@avasuro
Copy link
Contributor Author

avasuro commented Sep 23, 2023

Fixed

@melloware
Copy link
Member

@avasuro this is a fantastic PR. All tested and looks good on my end letting the CI Pipeline run then I will merge!

@melloware
Copy link
Member

ahh looks like you need to run npm run format to format the code.

@github-actions
Copy link

Thanks a lot for your contribution! But, PR does not seem to be linked to any issues. Please manually link to an issue or mention it in the description using #<issue_id>.

@melloware
Copy link
Member

The failures are unrelated to this change. Merging!

@melloware melloware merged commit bbc621c into primefaces:master Sep 23, 2023
2 of 5 checks passed
habubey added a commit that referenced this pull request Sep 27, 2023
@habubey
Revert "Fix #4940: scroll block applied incorrectly in some cases (#4946
3718ad0 
)"

This reverts commit bbc621c.
@habubey habubey mentioned this pull request Sep 27, 2023
Merged
@ulasturann
Copy link
Contributor

Hi @avasuro,

Thank you for your contribution through your PR. We've realized that the 'blockScroll' issue isn't related to your PR and have already addressed it. Could you please reopen your PR so that we can incorporate your valuable contribution into v10?

melloware added a commit to melloware/primereact that referenced this pull request Sep 27, 2023
@melloware
Fix primefaces#4946: Dialog scroll block tracking
f090f47
@melloware melloware mentioned this pull request Sep 27, 2023
Merged
melloware added a commit to melloware/primereact that referenced this pull request Sep 27, 2023
@melloware
Fix primefaces#4946: Dialog scroll block tracking
ed990d9
@melloware
Copy link
Member

@avasuro @ulasturann I submitted it in a new PR.

melloware added a commit to melloware/primereact that referenced this pull request Sep 27, 2023
@melloware
Fix primefaces#4946: Dialog scroll block tracking
b8ed1f2
melloware added a commit that referenced this pull request Sep 27, 2023
@melloware
Fix #4946: Dialog scroll block tracking (#4976)
819f423
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@melloware melloware melloware approved these changes

Assignees
No one assigned
Labels
Type: Enhancement Issue contains an enhancement related to a specific component. Additional functionality has been add
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Dialog: blockScroll + visible + fullscreen mode gives unexpected behavior if dialog is unmounted while visible
3 participants
@avasuro @melloware @ulasturann