Skip to content

Changed the minimum width of popup window #15984

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

Merged
merged 1 commit into from
Feb 10, 2022
Merged

Changed the minimum width of popup window #15984

merged 1 commit into from
Feb 10, 2022

Conversation

Ruslan-Aleev
Copy link
Collaborator

@Ruslan-Aleev Ruslan-Aleev commented Jan 21, 2022

What does it do?

For some reason the width of popups is sometimes reset to the minimum width.
Previously, the value was equal to 200px, which is not enough, increased to 400px, so that even when resetting the content is not cut off.

Set window width via parameter - #15984 (comment)

Before (content can be cut off by window):
trash

After:
minWidth

Why is it needed?

Prevent cut off of window content.

Related issue(s)/PR(s)

#14340

@cla-bot cla-bot bot added the cla-signed CLA confirmed for contributors to this PR. label Jan 21, 2022
@Ruslan-Aleev Ruslan-Aleev added pr/review-needed Pull request requires review and testing. proposal Proposal about improvement aka RFC. Need to be discussed before start implementation. type-frontend Issues related to UI/UX issues, mostly about styles and frontend implementations on JavaScript. labels Jan 21, 2022
@Ibochkarev Ibochkarev added this to the v3.0.0-rc2 milestone Jan 21, 2022
@Jako
Copy link
Collaborator

Jako commented Jan 22, 2022

Do you have a reproducible click path where this issue occurs? Then I can look for this. Does the alert has an explicit ID?

@Ruslan-Aleev
Copy link
Collaborator Author

@Jako This happens in the Deleted Resources Manager, see more details #14340

Does the alert has an explicit ID?

This happens, at least, by clicking on the "Erase All", "Restore All" button (most likely relevant for all windows). And you can click one, close it, and click another - the effect will repeat.

@Jako
Copy link
Collaborator

Jako commented Jan 24, 2022

This is not a real fix, because it does not solve but just bypass the issue.

@Ruslan-Aleev
Copy link
Collaborator Author

Yes, that's what I meant by:

It is more correct to look for the reason for resetting the width...

And added the "proposal" tag.

@Jako
Copy link
Collaborator

Jako commented Jan 24, 2022

Please use #15996 and set the minWidth of the confirm boxes by option.

MODx.msg.confirm({
    ...
    minWidth: 500,
    ...

@Ruslan-Aleev
Copy link
Collaborator Author

Ruslan-Aleev commented Jan 24, 2022

@Jako Are you sure it works?
Whatever width I set, the default width is taken for the window.
Checked at least in Deleted Resources Manager section.

@Jako
Copy link
Collaborator

Jako commented Jan 24, 2022

Yes it is working here fine. You have to add the minWidth to all MODx.msg.confirm calls in modx.grid.trash.js (6 times).

@Jako
Copy link
Collaborator

Jako commented Jan 24, 2022

And maybe you have to force reload the manager to load the changed javascripts.

@Ruslan-Aleev
Copy link
Collaborator Author

Yes, thanks, it works, I will correct this PR after your PR #15996 merges.

@Ruslan-Aleev Ruslan-Aleev marked this pull request as draft January 24, 2022 14:38
@opengeek opengeek modified the milestones: v3.0.0-rc2, v3.0.0-pl Feb 3, 2022
@Ruslan-Aleev Ruslan-Aleev marked this pull request as ready for review February 4, 2022 13:12
@Ruslan-Aleev
Copy link
Collaborator Author

@Jako please check.

@Ibochkarev Ibochkarev added pr/ready-for-merging Pull request reviewed and tested and ready for merging. and removed pr/review-needed Pull request requires review and testing. labels Feb 4, 2022
@opengeek opengeek merged commit 25005f9 into modxcms:3.x Feb 10, 2022
@Ruslan-Aleev Ruslan-Aleev deleted the 3.x-fix-14340 branch February 10, 2022 19:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla-signed CLA confirmed for contributors to this PR. pr/ready-for-merging Pull request reviewed and tested and ready for merging. proposal Proposal about improvement aka RFC. Need to be discussed before start implementation. type-frontend Issues related to UI/UX issues, mostly about styles and frontend implementations on JavaScript.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants