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

Refactor deprecated setModalLoader(); usage #34

Closed
luke- opened this issue May 1, 2023 · 9 comments
Closed

Refactor deprecated setModalLoader(); usage #34

luke- opened this issue May 1, 2023 · 9 comments
Assignees

Comments

@luke-
Copy link
Contributor

luke- commented May 1, 2023

No description provided.

@yurabakhtin
Copy link
Contributor

@luke- Core PR humhub/humhub#6389 - I have removed the deprecated JS function and implemented one new humhub.require("ui.modal").footerLoader() instead.

Then I have updated this module in the PR #35, but it requires new HumHub vesion 1.15, if it is not ok, I can try to update the JS code without using the new JS function footerLoader().

@luke-
Copy link
Contributor Author

luke- commented Jun 15, 2023

@yurabakhtin The footer loader of the updater is very special. Do you think its useful to provide a separate footerLoader() in the core, which is only used by the updater?

Wouldn't it be better to implement such a loader only in the updater?

Then we would not have problems with version dependencies.

@yurabakhtin
Copy link
Contributor

@luke-

The footer loader of the updater is very special. Do you think its useful to provide a separate footerLoader() in the core, which is only used by the updater?

It is used also in 3 core places:

So I moved the deprecated function setModalLoader() from static/js/humhub/humhub.ui.additions.js to static/js/humhub/humhub.ui.modal.js. Also I have deleted the function from the file static/js/humhub/legacy/app.js because I didn't find where it was used.

Wouldn't it be better to implement such a loader only in the updater?

Yes, I can implement/copy the same JS code from humhub.require("ui.modal").footerLoader() to this module updater, so we can revert the min version to 1.0 as it was before. Should I do this?

@luke-
Copy link
Contributor Author

luke- commented Jun 15, 2023

@yurabakhtin Sorry, missed that in my review. If the footerLoader(); is also used in the core, your approach is fine.

@luke-
Copy link
Contributor Author

luke- commented Jun 15, 2023

@yurabakhtin FYI, I've added the core change in the new migration doc: humhub/humhub@a2e708f - Please also add changes here.

@yurabakhtin
Copy link
Contributor

@luke-

FYI, I've added the core change in the new migration doc: humhub/humhub@a2e708f - Please also add changes here.

Do you mean to update the line Removed deprecated javascript method setModalLoader() somehow, or is it info for me to update the file in future when I will do some similar changes?

@yurabakhtin
Copy link
Contributor

yurabakhtin commented Jun 15, 2023

Yes, I can implement/copy the same JS code from humhub.require("ui.modal").footerLoader() to this module updater, so we can revert the min version to 1.0 as it was before. Should I do this?

@luke- Should I do something with this module "Updater" in order to don't increase the min version to 1.15?

@luke-
Copy link
Contributor Author

luke- commented Jun 19, 2023

@luke-

FYI, I've added the core change in the new migration doc: humhub/humhub@a2e708f - Please also add changes here.

Do you mean to update the line Removed deprecated javascript method setModalLoader() somehow, or is it info for me to update the file in future when I will do some similar changes?

It's just a info for you, for future similar changes.

@luke-
Copy link
Contributor Author

luke- commented Jun 20, 2023

Yes, I can implement/copy the same JS code from humhub.require("ui.modal").footerLoader() to this module updater, so we can revert the min version to 1.0 as it was before. Should I do this?

@luke- Should I do something with this module "Updater" in order to don't increase the min version to 1.15?

No, it's fine, I'll merge the PR. Thanks

@luke- luke- closed this as completed Jun 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants