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

Fixed observer missing parameter #428

Merged
merged 3 commits into from
Nov 8, 2024
Merged

Conversation

gevorgmansuryan
Copy link
Contributor

No description provided.

@gevorgmansuryan gevorgmansuryan requested a review from luke- November 7, 2024 18:32
@gevorgmansuryan gevorgmansuryan self-assigned this Nov 7, 2024
Copy link
Contributor

@yurabakhtin yurabakhtin left a comment

Choose a reason for hiding this comment

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

@gevorgmansuryan I think you should run grunt build for the Mail module to update the file resources/js/humhub.mail.messenger.bundle.js, becuase I see the file was updated in your previous PR.

Can we use a code observer.observe(this, instead of adding the element as param here function (index, element) {?

@gevorgmansuryan
Copy link
Contributor Author

@yurabakhtin updated pr.
yes we can use this, but with key value code is more readable in my opinion, this the essentially same thing as element in context of jquery each. is there any reason that you want to use this ?

@yurabakhtin
Copy link
Contributor

@gevorgmansuryan It is not important to use the this there, I just thought it takes less code chars :) and I see this way is used more often, but ok let keep as you wrote, thanks.

@luke- luke- merged commit 926f5d5 into master Nov 8, 2024
12 checks passed
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.

3 participants