@MHarmony opened this Pull Request on March 11th 2021 Contributor

Description:

I am currently developing a plugin that appends html to the top and bottom of the body. These two tags would be helpful in my plugin development.

Review

  • [ ] Functional review done
  • [ ] Potential edge cases thought about (behavior of the code with strange input, with strange internal state or possible interactions with other Matomo subsystems)
  • [ ] Usability review done (is anything maybe unclear or think about anything that would cause people to reach out to support)
  • [ ] Security review done see checklist
  • [ ] Code review done
  • [ ] Tests were added if useful/possible
  • [ ] Reviewed for breaking changes
  • [ ] Developer changelog updated if needed
  • [ ] Documentation added if needed
  • [ ] Existing documentation updated if needed
@sgiehl commented on March 12th 2021 Member

Hi @MHarmony thanks for creating the PR. The changes are looking fine.

@tsteur could you maybe have a quick look as well. Not sure if should add those new events or not. In general we should maybe try to avoid events where possible, as they might make Matomo slower and slower if we add too many. But guess those global events might be helpful at some point maybe.

@tsteur commented on March 14th 2021 Member

I think it be fine to add them 👍 What would always be helpful be to know how/for what they will be used @MHarmony . This will make it easier if/when there are changes/refactorings/... and what could break when making changes in the future there. If you can't tell us it's fine but it be good to know for us since for these twig events we don't consider API and we'd still want to make sure when/if there are any changes in the future to not break anything.

@MHarmony commented on March 18th 2021 Contributor

It's for notices at top/bottom of pages.

@tsteur commented on March 22nd 2021 Member

fyi @MHarmony I just documented our notifications classes. Just wondering if this might maybe work too? https://github.com/matomo-org/developer-documentation/pull/459/files

@MHarmony commented on March 22nd 2021 Contributor

@tsteur Unfortunately not. The notices are meant to be permanent, not toasts. Like a banner of sorts.

@tsteur commented on March 22nd 2021 Member

The notifications can be permanent too if triggered on every page load but all good. Just thought I mention it 👍

@MHarmony commented on March 22nd 2021 Contributor

@tsteur Thanks for the mention. I didn't know about these so this gives me something to use in the future even if it doesn't quite fit what I need here.

This Pull Request was closed on April 11th 2021
Powered by GitHub Issue Mirror