skyriter → created an issue.
skyriter → created an issue.
skyriter → created an issue.
skyriter → created an issue.
skyriter → created an issue.
skyriter → created an issue.
We sure can. It should already, based on the name, as they fire alphabetically if the weight's not been changed, but we can can also put our thumbs on the scale for good measure (although wouldn't that actually be "bad" measure)?
The top of the page alert message shows only when the block module is not enabled, per the sitewide_alert.module file:
/**
* Implements hook_page_top().
*/
function sitewide_alert_page_top(array &$page_top): void {
// Bail out if block submodule is enabled.
if (\Drupal::service('module_handler')->moduleExists('sitewide_alert_block')) {
return;
}
$page_top['sitewide_alert'] = \Drupal::service('sitewide_alert.sitewide_alert_renderer')->build();
}
To have the same functionality, we'll need to replicate this in the uswds_alert.module file.
The block module itself only creates a block, as it has 2 files:
- sitewide_alert/modules/sitewide_alert_block/src/Plugin/Block/SitewideAlertBlock.php
- sitewide_alert/modules/sitewide_alert_block/sitewide_alert_block.info.yml
There's nothing here that Drupal itself doesn't provide.
My recommendation is not to require the sitewide_alert_block submodule as a dependency.
The MR is ready for review.
Here's the current README:
**This is not a functional module yet.** It is under active development under [Contrib First](https://guidebook.civicactions.com/en/latest/common-practices-tools/contribution/contrib-first/).
And here's the help page:
<p><strong>This is not a functional module yet.</strong> It is under active development under <a href="https://guidebook.civicactions.com/en/latest/common-practices-tools/contribution/contrib-first/">Contrib First</a>.</p>
skyriter → created an issue.
skyriter → created an issue.
Requiring message
I agree about Alert Message not being required seems odd. I wouldn't want to make it required, though, given that we're using that pre-existing field, so that we are backwards compatible.
Order of Heading
We should be able to add Heading before Alert Message without issue.
Display order
I think we can still use Weight and only when it's the same as the another weight do we use the Display Order setting. We can add that to the help text for weight Weight.
Configuration
Where the config page shows is set in the routing file. I'd leave it be.
Field name
Given that this is a field on a custom entity type rather than a node, we're only going a conflict if we add another heading field to it.
I have added configuration and an install file to update the Alert Styles select list.
Here's what it looks like now:
skyriter → created an issue.
skyriter → created an issue.
skyriter → created an issue.
Ok, that makes sense.
I saw Steve's comment in the Mural document on the complexity of adding an "Alert Type" field and think I understand his counterproposal, but I want to verify here.
If I'm not mistaken, Steve, you're suggesting that we add all of the USWD styles to the "Alert Style" field and forego adding an "Alert Type" field. The implementation would look something like this on the module configuration screen:
For the user creating a sitewide alert, they would see something like the following:
skyriter → made their first commit to this issue’s fork.
skyriter → created an issue.
skyriter → made their first commit to this issue’s fork.
For my own clarity, are you thinking that the blocks are as follows:
Sitewide Alert
This is what comes "out of the box" with the sitewide_alert → module.
Sitewide Alert - USWDS Alert
- Alert - Informative
- Alert - Warning
- Alert - Success
- Alert - Error
- Alert - Emergency
What about Slim alert?
What about Alert with no icon?
Sitewide Alert - USWDS Site Alert
- Alert - Informational
- Alert - Emergency
- Alert - Emergency with no header
- Alert - Emergency with list
What about Slim site alert?
What about Site alert with no icon?
Sitewide Alert - Non-USWDS alerts
Why is would this type of alert a part of this module, given its purpose to provide USWDS alerts and site alerts?
Ok, that makes sense. I just added that requirement in the composer.json.
Do you think I should close this MR ✨ Add info.yml file Active and combine the work into one?
While this has been added, it could definitely use some beefing up.
skyriter → created an issue.
skyriter → created an issue.
skyriter → created an issue.
skyriter → created an issue.
skyriter → created an issue.
skyriter → created an issue.
skyriter → created an issue.
Ah, well, you can't exempt it as is not an unknown word but a "forbidden word," cue the scary music, per this https://www.drupal.org/project/drupal/issues/3364292 📌 Use 'queueing' spelling instead of 'queuing' Fixed .
I like the hook update idea. Good thinking.
skyriter → made their first commit to this issue’s fork.
Notice: Among other spelling issues, I updated the spelling of "queuing" to "queueing" (the Drupal approved spelling) in the config. I believe I caught them all.
skyriter → made their first commit to this issue’s fork.
skyriter → made their first commit to this issue’s fork.
skyriter → made their first commit to this issue’s fork.
OK, ESLint is now passing.
Updated to fix code style issues.
skyriter → made their first commit to this issue’s fork.
skyriter → made their first commit to this issue’s fork.
Updated ESLint issue and also removed jQueryQ.
Added exception words, as those were the only tripping up CSpell.
Made fixes to identified errors.
skyriter → made their first commit to this issue’s fork.
Updated with some good ole // @phpstan-ignore-next-line
skyriter → made their first commit to this issue’s fork.
skyriter → made their first commit to this issue’s fork.
I'lll pick this up.
skyriter → created an issue.
skyriter → created an issue.
skyriter → created an issue.
volkswagenchick → credited skyriter → .
skyriter → created an issue.
@jackfoust I posted a patch → on issue 2949540 ✨ Allow specific form ids for clientside validation Needs work for allowing us to exclude forms by form id so that we could use the browser's built-in validation. That is our current work-around.
Adding the newest patch, after the issues identified by coderdan → were resolved.
This patch removes any mention of webform.