🇺🇸United States @jcandan

A patch for 10.3.x sites. MR ready to be created if desired.

jcandan → made their first commit to this issue’s fork.

Good catch.

jcandan → created an issue.

jcandan → created an issue.

Adjust link colors. Apparently this is overridden in the d.o CSS causing links to be invisible until hover. This is not a good solution, but a temporary attempt at a fix for at least this page. Should probably submit an issue for a real fix.

Noted the functionality and linked to this issue from the Sub-Theme inheritance → docs.

Supplies link destination titles as link text.

Adds note about CKEditor Stylesheets behavior introduced in  Change record: New API for adding theme-specific styles in CKEditor 5 → . Links to existing issue to allow overrides from sub-theme.

Any reason this couldn't be submitted as a patch or merge request?

Go forward with an issue to get that Gitpod launcher working?

After reading https://git.drupalcode.org/project/drupal_cms/-/wikis/Contributing-to-Drupal-CMS and finding out that the trial experience is built on DDEV, I attempted to fire up Drupal CMS via DDEV Gitpod launcher.

It works. However, it does not support Project Browser out of the box, but not in the same way as 🐛 Fix Project Browser returns 0 results in Trial Experience Active . I get composer errors. I trudged through a couple, but hit a wall.

Not extensively tested other than that.

Steps followed:

1. Visit DDEV Gitpod launcher.
2. Enter `https://git.drupalcode.org/project/drupal_cms` as the git repository.
3. Delete the git repository with artifacts field.
4. Open Gitpod.
5. Replace config.allow-plugins: true with config.allow-plugins: { }.
6. Run ddev composer install and said yes to plugin install prompts except for tbachert/spi.

jcandan → created an issue.

mferanda → credited jcandan → .

The [node:field_mylimitedtermreferencefield:entity:url:path] worked perfectly. Thanks!

jcandan → created an issue.

I am playing with this same problem space. I am building an organization-specific USWDS (like design.va.gov), with a concept similar to how Emulsify demo’d Western U with multiple in-house brands. The plan is to have Storybooks at those multiple levels, and then at the individual project instance level (Drupal and non-Drupal projects).

So, the Drupal-specific twig muddies the waters.

Still playing. Curious how this pans out.

There is talk of forking Opigno. An effort is underway (see #14 → from another ticket), but it may not have gotten traction. However, with the release of 3.2.7, there may now develop a movement in the community to conduct a proper fork.

To avert this effort, to continue to foster community use, and to gain community contribution, please consider creating a development release for Opigno LMS and its ecosystem modules.

Drupal 10 compatible 3.2.7 has been released. With this release, setting to Needs Review to get community feedback to ensure Drupal 9 sites are now able to upgrade to Drupal 10 to close out this issue.

Thank you for this release! With the release of 3.2.7, setting to Needs Review to get community feedback to ensure Drupal 10 compatibility to close out this issue.

jcandan → created an issue.

jcandan → created an issue.

@andrewfarq,

Not sure what happened with [!5634], but it looks to have ended up only adding testSessionManagerInvalidateSessionOnDestroy(), and no other changes.

Also, your proposal in #40 🐛 Log out show error message "... your browser must accept cookies ..." Needs work may ignore other's experiences with this issue outside of the install process , such as after upgrade #21 🐛 Log out show error message "... your browser must accept cookies ..." Needs work or with anonymous users #22 🐛 Log out show error message "... your browser must accept cookies ..." Needs work . Or is that what #18 🐛 Log out show error message "... your browser must accept cookies ..." Needs work is proposing? I may have misunderstood that.

At non vanilla instance with firewall issues resolved, I am still getting "could not retrieve source count...".

Re-roll as suggested in #20 ✨ Import data from URL Closed: won't fix .

jcandan → changed the visibility of the branch 2927126-import-data-from to hidden.

::insert foot in mouth:: It turns out the CSV I was trying to reach was behind a firewall.

I've confirmed this functionality on a clean instance with a publicly available, remote URL https:// CSV.

Returning this to Closed (outdated). Cheers!

jcandan → changed the visibility of the branch 3469594-allow-a-remote to hidden.

Taking inspiration from patch #15 → of ✨ Import data from URL Closed: won't fix , with a fix to for the removal of file_save_data() → , it may be simpler to replace all the curl bits to go with something like:

    // Retrieve data from URL.
    $data = file_get_contents($remote_path);
    if ($data === FALSE) {
      throw new \RuntimeException("Unable to retrieve data from '{$remote_path}'.");
    }
    // Save data to temporary dir.
    $path = "temporary://" . basename($remote_path);
    $file = \Drupal::service('file.repository')->writeData($data, $path);
    if ($file === FALSE) {
      throw new \RuntimeException("Unable to save file to '{$path}'.");
    }
    return $file->getFileUri();

A patch rolled from #5 ✨ Allow a remote file to be specified for the Source path parameter Active .

This was closed 2 years ago. I did reopen, however I see ✨ Allow a remote file to be specified for the Source path parameter Active has a recent attempt at this. Setting back to closed, but as won't fix in lieu of that effort.

I disagree. Comment #16 ✨ Import data from URL Closed: won't fix simply points out that the s3:// stream wrapper works. External URLs are still broken.

Patch #15 needs a re-roll with the removal of file_save_data() → .

This is a core issue. Marking closed (duplicate). New core ticket: 🐛 Fix cropped handle icon Active .

jcandan → created an issue.

jcandan → created an issue.

So far as I can see on a cursory review, the set of Icons determined to be available are hard-coded here and here (Not sure why it is listed twice, and each list is different).

I imagine we could refactor this to a single place. Then ensure that place gets the list from configuration. There may also probably need to be a mapping from the above configuration to the iconSource options listed at the vanila-icon-picker NPM package docs.

jcandan → created an issue.

thejimbirch → credited jcandan → .

If it really is a breaking change, that warrants a major bump. But, I’m beginning to think it is not a breaking change. The controller and route were in place because the view had not controlled that without a page display. With this change, it does. It also shifts access control to the view with a new view access plugin. So, all the update is confined to the one view.

With a hook update, we could override any customizations they may have made to the view. Without the hook update, if they don’t run the view update config:set, they won’t get the changes and it’ll break.

I’m of the opinion that if they customize a module-provided view, any config changes upon update is there’s to deal with.

That said, I’m for a minor update to 1.1.0 and a hook update to close out this ticket.

Still awaiting community feedback on options 1 or 2 noted in #13. 🐛 Fix Bookmarks view is missing a display Needs review

This also solves a previously hidden views permissions issue which prevented non-admin users from seeing the links and title fields reported at 🐛 Fix bookmark link not shown for non-admin users Fixed #8 🐛 Fix bookmark link not shown for non-admin users Fixed .

Apologies, I should have noted this here before. The bug described in #8 and #18 should be resolved by 🐛 Fix Bookmarks view is missing a display Needs review .

One thing I noticed prior to the patch, this only affected one of our two migrations with the above uid field process configurations. Not sure why that is.

Either way, patch #2 applied successfully on 6.0.4 and warnings have stopped. Thanks!

jcandan → created an issue.

Leaving this one last note here for if we do decide to go with a hook update.

function bookmarks_update_VERSION() {
  $config_factory = \Drupal::configFactory();
  $view = $config_factory->getEditable('views.view.bookmarks');

  if ($view->get('display.default.display_options.access.type') != 'bookmarks_view_access') {
    $view->set('display.default.display_options.access.type', 'bookmarks_view_access');
    $view->set('display.default.display_options.title', 'Bookmarks');
    ...
    $view->save();
  }
}

As we consider this, these resources may help with development:

• function hook_update_N
• Don’t go making major version changes
• 📌 Document how contrib hook_update_N() should be numbered now that modules can be compatible with multiple major branches and versioned semantically Fixed

We've adopted patch #12 to a production project, I am happy to place this to RTBC; however, I am chewing on whether this needs a hook update for the veiws.view.bookmarks changes and whether this would need a major, minor, or patch version bump.

Obviously, the fix is adopted for any new install. But, for this fix to apply to existing installations would override their existing Bookmarks view--this could be considered a breaking change requiring a major version bump.

The way we applied the change was to simply import the single updated configuration from this module, and then used git to help us track which changes we wanted to keep:

cat web/modules/contrib/bookmarks/config/optional/views.view.bookmarks.yml | drush config:set --input-format=yaml views.view.bookmarks ? -

I imagine we could just bump the patch version and include this instruction in the release notes.

So, the above boils down to, either we:

1. Include a hook update and bump the major version.
2. Include the single config import instructions in the release notes and bump the patch version.

Opinions welcome.

Fixes a bug with Bookmarks tab not showing.

I'd also like to rename the Views Access plugin.

I guess I need to update the target branch. Considering there will be a hook_update_N(), this should bump minor version from 1.0.x to 1.1.x, right?

I was able to solve the views access constraint noted in #7.

It still needs a hook update for the views changes. For now, I pushed this patch and MR up to get community feedback.

Anyone applying this patch wanting to adopt the views config changes may run the following:

cat web/modules/contrib/bookmarks/config/optional/views.view.bookmarks.yml | lando drush config:set --input-format=yaml views.view.bookmarks ? -

One caveat to note: Due to 🐛 Fix incompatibility with Views Enity Form Field Needs work , one is required to add the draggable view as a separate view (either as an attachment or on another page), listing the bookmarks twice (once for editing titles and another for sort). Hopefully they get that sorted out some day.

I think we should support 8.8+. Thank you for the contribution!

Set this back to Fixed to let the 2 week automation → play out.

Set this back to Fixed to let the 2 week automation → play out.

jcandan → created an issue.

Pushed patch to fork with spaces in place of tabs.

jcandan → made their first commit to this issue’s fork.

After some digging, to replace the access checks removed, this patch's view is going to need the following permissions checks:

• access others bookmarks
• Current user is viewing own bookmarks list

It turns out that the recommendation is to not publish a development release → . I will close this issue, since it is not a fast moving project that is almost ready to be marked feature complete and minimally maintained (not bad things to be, by the way). If the community wishes to re-open this issue, do comment.

It turns out that I only couldn't create a new branch due to a known 💬 Can't create new branch in Gitlab Closed: works as designed issue → with Drupal Gitlab.

It also turns out that the recommendation is to not publish a development release → . I will close this issue, since it is not a fast moving project that is almost ready to be marked feature complete and minimally maintained (not bad things to be, by the way). If the community wishes to re-open this issue, do comment.

jcandan → created an issue.

Seems I missed a bug. After applying the patch, the links and title fields don't show in the Bookmarks list table. Digging.

Patch #6 fixed the issue for us on Drupal 10.3.0 with Bookmarks 1.0.2.

@bkline, would you agree that a later ticket could supersede an existing ticket due to some combination of similarity, clarified direction, and traction? In that case, if not "Closed (duplicate)", what alternative?

Just on a cursory review, it seems that #3278410: block_theme_initialize() sledgehammers square blocks into round regions, allow themes to provide a mapping of regions to 'roles' → was spun off from 📌 block_theme_initialize() falls back to putting blocks in the first region it finds, provide a better default Needs work , and these tickets combined are attempting to organize a solution. Is either ticket deficient in some way? Do you have a solution that supersedes the efforts bubbling up in those tickets? If not, marking this ticket as a duplicate may just be a way of, hopefully, limiting duplicate efforts.

Also, it turns out the original ticket, 📌 block_theme_initialize() falls back to putting blocks in the first region it finds, provide a better default Needs work , was created 1 day prior to this one.

Not making an argument for either ticket--haven't dug in enough to have an opinion on that. I do wonder, though,--but don't have time to dig in just now--are any of these efforts a spin off of #37 🐛 Sub-theme doesn't inherit base theme default block config Closed: duplicate ? :)

A patch supporting the Name field.

Using the existing constraint, if I could just get it to run the AddressFormatConstraintValidator before RequireOnPublishValidator, I think I would then be able to alter the $this->context->getViolations.

Tried increasing the module weight with module_set_weight() to no avail.

I was able to Disable HTML5 validation → to get around the Address module's required attribute. So, similar to Name, I am getting the required subfield validation.

I wonder if this module will play well with others. Take a look at Require on Publish → .

Just had an idea, noting it here so I don't forget.

What if I tackle this from a new Constraint?

Gave this a shot.

/**
 * Implements hook_field_widget_complete_form_alter().
 */
function require_on_publish_field_widget_complete_form_alter(&$field_widget_complete_form, FormStateInterface $form_state, $context) {
  $field_config = $context['items']->getFieldDefinition();
  $field_name = $field_widget_complete_form['widget']['#field_name'];
  /** @var \Drupal\Core\Entity\EntityFormInterface $form_object */
  $form_object = $form_state->getFormObject();
  if ($form_object instanceof EntityFormInterface) {
    /** @var \Drupal\Core\Entity\ContentEntityInterface $entity */
    $entity = $form_object->getEntity();
    if (_require_on_publish_entity_is_publishable(get_class($entity))) {
      // $field = $entity->getFields()[$field_name];
      // $field_config = $field->getFieldDefinition();
      if (($field_config instanceof FieldConfigInterface)) {
        if ($field_config->getThirdPartySetting('require_on_publish', 'require_on_publish', FALSE)) {
          $place_breakpoint_here = 'Analyze the available Widget';
          // Loop the widget sub-fields and determine if they're required. Then,
          // treat each required subfield as if it were required on publish.
        }
      }
    }
  }
}

Tried similar attempts from preprocess hooks, like require_on_publish_preprocess_form_element(). The problem is that I cannot get a combination of render array and the entity context necessary to alter the render array to:

1. Determine the subfield's parent is required on publish.
2. Remove the required attribute.
3. Somehow implement the require_on_publish logic.

I am open to suggestions and willing to accept DMs in #slack.

Original report

The original report proposed that all subfields should be marked optional when reproducing this issue. It then suggested that the field could be subsequently submitted as Published without the desired required fields. This makes sense because there was no field marked as required. ROP validation kicks in if the field is left completely empty.

For the Address field, this means that to not have selected a Country, it would not pass Require on Publish validation. And it correctly does not.

For the Name field, it does not even allow you to set all subfields as optional, at least Given or Family must be required:

If I were to fill in the Given and Family fields, and Select a Country, this is able to be saved as Published.

Updated title to clarify that this isn't about cardinality, but fields types that include multiple fields--compound fields.