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

Fix #1080 Materialize Upgrade #1160

Closed
wants to merge 2 commits into from

Conversation

guido97
Copy link
Contributor

@guido97 guido97 commented Oct 21, 2021

Closes #1080

Description

Change use for modals methods, fix calls to toast, use AutoInit(),
The automatic initialization has been removed in Materialize Version 1

In general, update according to official guide and test changes
https://github.com/Dogfalo/materialize/blob/v1-dev/v1-upgrade-guide.md

Change use for modals methods, fix calls to toast, use AutoInit(),
The automatic initialization has been removed in Materialize Version 1

In general, update according to official guide and test changes
https://github.com/Dogfalo/materialize/blob/v1-dev/v1-upgrade-guide.md
@brylie
Copy link
Member

brylie commented Oct 21, 2021

@guido97, I'm having difficulty reviewing these changes since there are a lot of changed lines in the template files. Are there some particular aspects of the templates that needed to change?

@gorkemarslan just merged a similar pull request. Were the changes similar to this one that fixes the Materialize errors?

@guido97
Copy link
Contributor Author

guido97 commented Oct 21, 2021

Hi @brylie, my changes are for finish update to the version 1.0.0 of materialize (Latest release), the most of these changes are for calls to toast. The version 0.97.6 is from April 1, 2016, perhaps works, but is oldest.
In this new version automatic initialization was removed, but we can use Materialize.AutoInit (), therefore some initializations I commented. in the image below you can see modal and toast working

screenshot-modal_and_toast

@gorkemarslan
Copy link
Collaborator

gorkemarslan commented Oct 22, 2021

@guido97, as @brylie says, it is going to a bit difficult to follow changes in templates and JS codes.

Issue #1080 appeared after we removed dependencies folder and change the static Django template tags with related CDN, as can be seen in issue #1051. So our initial purpose here should be to fix the problem on the similar base and should be simple as in PR #1080, Upgrading materialize framework from 0.97.6 to 1.0 can be considered as a further step after PR #1080. But we try to give weight to using Django templates if possible and reducing code dependencies on JS. (See similar discussion in #1129).

I am skeptical to upgrade Materialize here. You notice upgrading from 0.100.2 to 1.0. That's nice. However, there should be some changes from 0.97.6 to 0.100.2. As far as I see, there is lack of tutorials for the older versions of materialize on the Internet. So upgrading can cause further side effects that can be detected harder because someone has to check all pages and JS codes to make sure there is no problem. This needs some time and energy, of course.

Keeping simple and focusing on fixing opened issues should be our strategy.

Could you check current develop branch work well with PR #1080? If you find new issues, could you report them by creating a new Issue?

@brylie brylie marked this pull request as draft October 26, 2021 10:49
@gorkemarslan
Copy link
Collaborator

Fixed by #1158

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.

Materialize is not defined
3 participants