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

V11 compatibility #59

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

Conversation

Maravedis
Copy link

Just making the module compatible with v11:

  • Removed Suppress window size warning
  • Removed Hot reload, now a foundry feature
  • Corrected deprecation warnings.

The only real loss is the "Suppress window size warning" feature. Unfortunately, I didn't find a way to keep it with the new architecture. There might be a way with a very convoluted highjacking of the issues singleton or a hacky watch of the notifications singleton, but it didn't look like a good way to go about it.

Unfortunately, in the v11 update, the method [Game._dispkayUsabilityErrors](https://foundryvtt.com/api/v10/classes/client.Game.html#_displayUsabilityErrors) has been removed.
In its place, there is now a dedicated singleton, called [ClientIssues](https://foundryvtt.com/api/classes/client.ClientIssues.html) that handles this kind of warnings.
Most of ClientIssues fields are private and cannot be altered in anyway, so this feature is not possible in v11 for now.

Removing this feature removes the lib-wrapper dependency, so it also has been removed by this commit.
@Maravedis
Copy link
Author

This PR is heavily inspired by @Eunomaniacs in #57 .

@Eunomiac
Copy link

Eunomiac commented Jan 23, 2024

The only real loss is the "Suppress window size warning" feature. Unfortunately, I didn't find a way to keep it with the new architecture. There might be a way with a very convoluted highjacking of the issues singleton or a hacky watch of the notifications singleton, but it didn't look like a good way to go about it.

Well, that damn notification irritated me so much I spent a whole evening learning about Proxy objects and fiddling with things, but I got it to work! Here's the fully-commented code; you can paste it directly into ../modules/_dev-mode/foundryvtt-devMode.mjs at line 13, right after the imports. (You can also take the CSS generated by the ApplyHideNotificationStyles function, and instead incorporate it directly into ../modules/_dev-mode/foundryvtt-devMode.css, but I'd already done it this way when I realized the module had its own css file :) )

It's scalable to any notifications that one might want to silence (both in the console and the UI) --- I've hard-coded in the two I want muted, but this could easily be extended via settings into giving users the ability to customize exactly which notifications they want to hide.

/**
 * This function applies a style to the <head> element to hide notifications in the UI
 * that have been intercepted by the `InitNotificationsProxy` function.
 *
 * This function should be run during the "init" Hook call.
 */
function ApplyHideNotificationStyles() {
    // Create a style element to hide intercepted notifications in the UI.
    const style = document.createElement("style");
    style.textContent = "#notifications .do-not-display { visibility: hidden; }";
    // Append it to the <head> element
    document.head.appendChild(style);
}

/**
 * This function initializes a Proxy to intercept changes to the `ui.notifications.queue` array.
 * It accepts an array of strings to match against any notification that would be added to the queue.
 * If a notification matches, the Proxy will modify it to prevent it from logging anything to the
 *  console, and to hide it in the UI.
 *
 * This function should be run during the "ready" Hook call.
 */
function InitNotificationsProxy(patternsToHide = []) {
    // Do nothing if no patterns are provided.
    if (!patternsToHide.length) { return; }

    // Get the original array of queued notifications, and store a constant reference to it
    const notificationQueue = ui.notifications.queue;

    // Convert the provided patterns into regular expressions
    const regExpPatterns = patternsToHide.map((pattern) => new RegExp(pattern));

    // Define a handler for the proxy that will be used to intercept notifications
    const handler = {
        set: function (target, property, value) {
            // Handle changes to the array length property
            if (property === "length") {
                // Perform the default behavior for length changes
                target.length = value;
                return true; // Indicate success
            }
            // Handle directly setting the value for non-index properties (necessary for array methods like 'next')
            else if (typeof property === "string" && isNaN(Number(property))) {
                // Perform the default behavior for non-index properties.
                target[property] = value;
                return true; // Indicate success
            }
            // Handle setting array indices
            else if (!isNaN(Number(property))) {
                // If the value is a notification and its content matches one of the provided patterns ...
                if (value
                    && typeof value === "object"
                    && "message" in value
                    && typeof value.message === "string"
                    && regExpPatterns.some((pattern) => pattern.exec(value.message))) {
                    // ... edit the notification to:
                    Object.assign(value, {
                        console: false, // ... prevent logging it to the console
                        permanent: false, // ... ensure the notification element is removed automatically
                        type: "do-not-display" // ... 'hack' the type to add the 'do-not-display' class
                    });
                }
                // Otherwise, perform the default behavior for setting index properties.
                target[Number(property)] = value;
                return true; // Indicate success
            }
            return false; // Indicate failure for all other cases
        }
    };

    // Replace the notifications queue array with a Proxy defined by the above handler.
    ui.notifications.queue = new Proxy(notificationQueue, handler);
}

// Apply the <style> element that will hide the UI notification during the 'init' hook
Hooks.once("init", ApplyHideNotificationStyles);

// Initialize the notifications proxy during the 'ready' hook, after ui.notifications has been defined
Hooks.once("ready", () => {
    // I've hard-coded the two notifications I want to hide, but this could easily be a
    //   user setting, allowing users to customize which notifications are silenced.
    InitNotificationsProxy([
        "Foundry Virtual Tabletop requires a minimum screen resolution",
        "not displayed because the game Canvas is disabled"
    ]);
});

I tried simply not adding the notification to the Proxy array (instead of editing its settings and relying on dynamically-generated <style> CSS), but that caused issues with certain core methods of the Notifications class --- I think it expects a notification to be there once it's added it, and gets confused when it can't find it!

I've been running this through its paces for the past couple of hours and it works very smoothly, without impacting any other behavior of ui.notifications or the Notifications class --- at least, as far as I can tell!

(I also considered making my own fork and pull request to add this, but I saw you had already made one and it hadn't been merged into the original, so figured it would be easier to keep all of our "v11" fixes in one place!)

@Maravedis
Copy link
Author

I think this would legit work as a stand-alone module that would allow you to fine-tune muting any notifications, with regexes or even a pre-determined list.

I'll see about incorporating this in the MR soon-ish.

@kaelad02
Copy link

The module.json file should also be converted to the new format introduced in v10.
https://foundryvtt.com/article/manifest-migration-guide/

@misthero
Copy link

I've been running this through its paces for the past couple of hours and it works very smoothly, without impacting any other behavior of ui.notifications or the Notifications class --- at least, as far as I can tell!

(I also considered making my own fork and pull request to add this, but I saw you had already made one and it hadn't been merged into the original, so figured it would be easier to keep all of our "v11" fixes in one place!)

I did a tiny configurable module using your code: https://github.com/misthero/warning-remove

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.

4 participants