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

Add an ability to set and reset Toastify options globally #803

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Antreesy
Copy link

☑️ Resolves

  • Add setGlobalToastOptions(options: ToastOptions) function
  • Add resetGlobalToastOptions() function
  • Update README.md
  • Fix typos, remove redundant comments
  • Close Global toastify options #380

🚧 Tasks

  • Code review
  • ⛑️ Tested in sandbox and Talk
  • 📘 API or User documentation - should be updated?

Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
@Antreesy Antreesy added documentation Improvements or additions to documentation enhancement New feature or request labels Apr 28, 2023
@Antreesy Antreesy self-assigned this Apr 28, 2023
Signed-off-by: Maksim Sukharev <antreesy.web@gmail.com>
@ChristophWurst
Copy link
Contributor

This makes it still local to the app

@Antreesy
Copy link
Author

This makes it still local to the app

That shouldn't be the problem, if we're going to define a container separately for each application.

For example, in Files app container is <main id="content" class="app-files">,
when in Talk it is <div id="content-vue" class="content app-talk">.

So if we manage to set the global selector for the whole NC instance, it's still needs to be manually changed for Talk, which makes not much sense in that case

@ShGKme
Copy link
Contributor

ShGKme commented May 1, 2023

This makes it still local to the app

The question is, do we want to make it globally across the apps?

  1. On one hand, it may solve a problem then toasts from one app don't work because of the layout of another app.
  2. On the other hand, in this case, one app changes the behavior of another app in a possibly unexpected way.

I'm not sure if p.1 is an actual problem.

What about to start with a module-wide global and then if there will be a case, to add a fully global option?

@nickvergessen
Copy link
Contributor

On one hand, it may solve a problem then toasts from one app don't work because of the layout of another app.

That is actually one of the problems we try to solve.
Currently when the Talk fullscreen option is used, the integrations of deck, tasks, etc. all miss their toasts as they are still displayed on the body, instead of the fullscreened container.

@ShGKme
Copy link
Contributor

ShGKme commented May 1, 2023

Then can we store the value for the default selector in the window, similar to @nextcloud/l10n?

https://github.com/nextcloud/nextcloud-l10n/blob/186d34ae2eb53cab52779306d5d52cea35966acc/lib/registry.ts#L69

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Global toastify options
4 participants