🇺🇸United States @cosmicdreams

Minneapolis/St. Paul
Account created on 9 December 2005, over 18 years ago
  • Software Engineer at Nerdery 
#

Merge Requests

More

Recent comments

🇺🇸United States cosmicdreams Minneapolis/St. Paul

Oh wait, I'm not actually seeing any change in your MR @mandclu

🇺🇸United States cosmicdreams Minneapolis/St. Paul

Yes, but I need to write a test for that.

🇺🇸United States cosmicdreams Minneapolis/St. Paul
🇺🇸United States cosmicdreams Minneapolis/St. Paul
🇺🇸United States cosmicdreams Minneapolis/St. Paul

@bsnodgrass in recent work, I'm working on making this setting be an exportable setting. I'm going to mark this issue as a duplicate of that one.

🇺🇸United States cosmicdreams Minneapolis/St. Paul

we are compatible with D10

🇺🇸United States cosmicdreams Minneapolis/St. Paul

I'm unsure if this issue persists (after our recent linting of the javascript). I am able to open the new window properly.

I can do all of the steps when using a new drupal site that uses the umami installation profile.

Are you still seeing this issue?

🇺🇸United States cosmicdreams Minneapolis/St. Paul

All done now.

🇺🇸United States cosmicdreams Minneapolis/St. Paul
🇺🇸United States cosmicdreams Minneapolis/St. Paul

I think I see the reason why we have avoided creating this schema. We want to make sure we aren't impacting the localStorage's management of this data. I'll have to manually test (and hopefully create automated tests) to ensure we don't have any weird interactions.

🇺🇸United States cosmicdreams Minneapolis/St. Paul

Trying to set attribution properly.

🇺🇸United States cosmicdreams Minneapolis/St. Paul

I've got a change that maintains functionality but should keep from hitting the svg tag itself. Only using classes now.

🇺🇸United States cosmicdreams Minneapolis/St. Paul

cosmicdreams changed the visibility of the branch 3451204-prevent-component-css to active.

🇺🇸United States cosmicdreams Minneapolis/St. Paul

cosmicdreams changed the visibility of the branch 3451204-prevent-component-css to hidden.

🇺🇸United States cosmicdreams Minneapolis/St. Paul

cosmicdreams created an issue.

🇺🇸United States cosmicdreams Minneapolis/St. Paul

Hello all, I found this issue while trying to understand the issues I'm having throughout my site's use of text editors and why it is no longer filtering out spaces (&nbsp).

I think most of the issue I'm seeing can be explained by this discovery, documented https://www.drupal.org/project/drupal/issues/3409587#comment-15366524 🐛 [10.2 regression] RSS feeds invalid due to   Fixed

an excerpt:

Drupal 10.1:

print \Drupal\Component\Utility\Html::normalize("<p>&nbsp;</p>");
<p> </p>

 print \Drupal\Component\Utility\Html::normalize("<p>\xc2\xa0</p>");
<p> </p>

Drupal 10.2:

print \Drupal\Component\Utility\Html::normalize("<p>&nbsp;</p>");
<p>&nbsp;</p>

print \Drupal\Component\Utility\Html::normalize("<p>\xc2\xa0</p>");
<p>&nbsp;</p>

So now I'm looking for any issue that might seek to address this issue in Html::normalize and I found this issue.

Are you aware of this issue?

🇺🇸United States cosmicdreams Minneapolis/St. Paul

I'm trying to follow along here because I was considering creating a new ticket but this one sounds a lot like my issue.

I'm trying to create a story that works with Storybook's arguments / controls. I can get a simple argument to work, but I can't get an argument that works with a slot to work.

I'm eager to see an example that demonstrates it's possible.

🇺🇸United States cosmicdreams Minneapolis/St. Paul

The work is back onto me to figure out remaining phpstan issues.

🇺🇸United States cosmicdreams Minneapolis/St. Paul

Is the goal of the statistics module to provide useful tools to augment a Google Analytics-type data analysis or is it to provide telemetry of use of the site to augment a Splunk-like data analysis?

🇺🇸United States cosmicdreams Minneapolis/St. Paul

As @brianperry has shown, yes. And we agree that's a great first step.

It should be noted, that the Next Steps are still on the Same Page Preview team.

We need to demonstrate what the actual change would be to include this into core. That said, before Starshot, the typical first step would be to be included as an experimental module. And for that step, same_page_preview would likely change very little. We do anticipate some change due to feedback / core contributor's direction. Fundamentally thought, we've already tried to only use core and to do as little as possible.

But if this work was in core, some obvious changes would be made. Such as:

  • No longer need to extend @internal objects
  • spread our logic to existing administrative forms
  • As a result, probably won't need to any any new objects, forms, or other objects. Just extend existing ones with additional logic.

In addition, I'm personally very excited about the new navigation bar and the top navigation bar specifically. We have an excellent opportunity to demonstrate the utility of both with our preview pane. Eager to learn more about making use of it.

🇺🇸United States cosmicdreams Minneapolis/St. Paul

In your example, I notice that you're using the new components you've added like heading and image. Should I also be doing that (instead of what the previous story did which was use scalar values for each)?

🇺🇸United States cosmicdreams Minneapolis/St. Paul

Yes it does. Thank you. I should have scanned the rest of the components in the module. I did find heading and image but didn't think of looking for anything that didn't start with my

🇺🇸United States cosmicdreams Minneapolis/St. Paul

It's fair.

Here's what I think:

1. We create a new version that's should be the one that people get for new Drupal versions.
2. We keep the current version "stuck in the past", when sdc was experimental.

🇺🇸United States cosmicdreams Minneapolis/St. Paul

I've been going through this process but I'm not sure how to do the embed / include that embeds another component

Like how my-banner--tall, with Card story does.

I'm also, kind of stalled out considering what methods should actually be used during a story. I could embed the card like the template does but I think that might be cheating. Don't we want to include the component's template and tell the my-banner--tall component to include the card.

In the yml file the story looks like this:

  - name: 4. With Card
    args:
      heading: Join us at The Conference
      ctaText: Register
      ctaHref: https://www.example.org
      image: ''
      banner_body: |
        {% embed 'sdc_examples:my-card' with { header: 'Protect what you love' } %}
          {% block card_body %}
            <p>Green technology is kind to the environment. Let's build a humanity that thrives while being kind to the planet.</p>

            {% include 'sdc_examples:my-button' with { text: 'Like', iconType: 'like' } %}
          {% endblock %}
        {% endembed %}

So the banner_body doesn't quote the embed syntax, So... not sure what to do.

Any suggestions?

If it helps here's my my-banner--tall.stories.twig so far:

{% stories my_banner with { title: 'SDC Examples/My Banner' } %}

    {% story default with {
        name: '1. Default',
    } %}
        <div class="container">
            {{ include('sdc_examples:my-banner', {heading: 'Join us at the Conference', image: 'https://picsum.photos/seed/affse3/1200/150', ctaText: 'Register', ctaHref: 'https://www.example.org'}) }}
        </div>
    {% endstory %}

    {% story no_image with {
        name: '2. No Image',
    } %}
        <div class="container">
            {{ include('sdc_examples:my-banner', {heading: 'Are you sure you want to end the session?', image: '', ctaText: 'Log out', ctaHref: 'https://www.example.org'}) }}
        </div>
    {% endstory %}

    {% story with_body with {
        name: '3. With Body',
    } %}
        <div class="container">
            {{ include('sdc_examples:my-banner', {heading: 'Improve perceived performance', image: 'https://picsum.photos/seed/se3/1200/350', ctaText: 'Learn More', ctaHref: 'https://www.example.org', banner_body: "<h4>Benefits of our technology</h4>
        <ul>
          <li>Faster first contentful paint.</li>
          <li>Distributed content delivery networks.</li>
          <li>High availability and redundancy.</li>
        </ul>"}) }}
        </div>
    {% endstory %}

{% endstories %}
🇺🇸United States cosmicdreams Minneapolis/St. Paul

If Same Page Preview was in core, there would be a number of changes we would do.

  • There are number of places we are extending core: NodeForm, PreviewControlsForm. We wouldn't need to do that and instead would refine our overrides further and include them in the Core objects.
  • CSS: We've had to include a small amount of changes to styles the we may not need to do if we could teach a future off-canvas dialog how to be resized without artifacts.
  • JS: We anticipate that much of the work of including this module into core is going to be spent refining the javascript. There's totally room for improvement. And in a world where experience builder and other dynamic user experience also exist, there's probably a lot of new ways of accomplishing what we're doing.

If it helps, we could produce an MR that would show what including this into core would look like.

🇺🇸United States cosmicdreams Minneapolis/St. Paul

From reading through this issue it sounds to me that the Next steps here are:

Next Steps

  1. Figure out how to add an '#html' property to fields
  2. Use that key to demonstrate how we could implement existing AJAX commands with HTMX
  3. Brainstorm a proof of concept scenario that would use those altered commands

Do I have that right?

🇺🇸United States cosmicdreams Minneapolis/St. Paul

cosmicdreams created an issue.

🇺🇸United States cosmicdreams Minneapolis/St. Paul

Original issue meaning the large buttons?

🇺🇸United States cosmicdreams Minneapolis/St. Paul

Cool. Seems like we're make progress.

previewOnHiddenField is indeed defined in same-page-preview.js, line 18.
It is used in only 1 place, when initializing our code.

But perhaps it is the selector we have chosen to use that Safari takes objection with (since it retrieve null, before we invoke a method on it).

The full line of code is previewOnHiddenField = document.querySelector('[data-drupal-selector="edit-ssp-preview-enabled"]'),

Next Steps

  • Test the selector in isolation on Safari
  • Add defensive programming tatics that detects if object is null and attempt secondary object retrieval logic if needed.
🇺🇸United States cosmicdreams Minneapolis/St. Paul

Oh sorry @DieterHolvoet I thought I was updating a different issue. I see the patch included. I'll give that a good test.

🇺🇸United States cosmicdreams Minneapolis/St. Paul

I've taken a look at how same page preview is working with Safari 17.4.1 and I can reproduce the bug. It took me a bit to get dev tools open so I can view the javascript console log.

Whatever this error is, it is hidden by the number of TypeError messages that Safari is giving out. Here's a list of files Safari has errors with:

  • toolbar.js
  • MenuModel.js
  • ToolbarModel.js
  • BodyVisualView.js
  • MenuVisualView.js
  • ToolbarAuralView.js
  • ToolbarVisualView.js
  • MenuVisualView.js

None of these files have anything to do with our module. In fact, Safari didn't log one message; Error, Warning or otherwise about Same Page Preview.

It's already odd, that whatever error is in our way here is only happening on Safari. But now it isn't clear if Safari has an issue with our module or if it's just errors with Drupal's new toolbar.

🇺🇸United States cosmicdreams Minneapolis/St. Paul

I'm noticing that the issue is a bit worse that mentioned in the testing steps.

We're not preserving the view mode at any step. I wonder if we were before.

This mechanism depends on our storing of configuration in localStorage. I'll inspect if that's working properly.

🇺🇸United States cosmicdreams Minneapolis/St. Paul

We believe this is resolved in recent work that clears a specific cache on install. We have yet to release that in 2.1.3 but it will be soon.

🇺🇸United States cosmicdreams Minneapolis/St. Paul

I can't seem to reproduce this. Here are my testing steps:

  1. Using same_page_preview 2.1.2, install a drupal site with the demo_umami profile
  2. Install same_page_preview (with drush if that matters)
  3. With same_page_preview installed edit some content
  4. Uninstall same_page_preview
  5. Observe that all of the views still exist

I'll keep this issue open for a bit for you to provide your own testing steps on how to reproduce the bug. I'll close this in a week or so if no one can provide a solid test case where this bug occurs.

🇺🇸United States cosmicdreams Minneapolis/St. Paul

I'm marking this issue as postponed until the issue resurfaces.

🇺🇸United States cosmicdreams Minneapolis/St. Paul

Considering adding this to May sprint

🇺🇸United States cosmicdreams Minneapolis/St. Paul

Removing SDC doesn't appear to have an impact on newer version of Drupal. (works in starshot as is). Perhaps if it were to cause a problem in newer version of Drupal, that is when we should consider removing it.

In other words, what is the harm in keeping our requirements as is?

🇺🇸United States cosmicdreams Minneapolis/St. Paul

I wonder if this issue is fixed in the latest version (and it's collapsible buttons)
@mandclu do you agree?

🇺🇸United States cosmicdreams Minneapolis/St. Paul

We have been considering adding this feature in our final phase. We've been debating different approaches since the beginning. We haven't made a lot of progress though.

I think the current plan is to see if we can leverage Composable's https://www.drupal.org/project/decoupled_preview_iframe to be able to declare what url should be the target for the preview.

Now that the experience builder is being built, they might find themselves building a far better solution that what can be accomplished here. We hope to experiment with ideas until we land on something workable here.

🇺🇸United States cosmicdreams Minneapolis/St. Paul

I got sign-off on this change in slack (#preview channel if you want to jump in.)

🇺🇸United States cosmicdreams Minneapolis/St. Paul

For me, this small fix is not enough. I have an additional change I'd love to see in this fix. @mandclu may I extend your MR with it?

🇺🇸United States cosmicdreams Minneapolis/St. Paul

Previously I was overriding the title with of an off-canvas dialog like this:

/**
   * Overrides \Drupal\Core\Entity\Controller\EntityViewController::title().
   *
   * Set preview pane title to "Preview" always.
   *
   * @param \Drupal\Core\Entity\EntityInterface|NULL $node_preview
   *   The node preview entity.
   *
   * @return \Drupal\Core\StringTranslation\TranslatableMarkup
   *   Title of the preview pane.
   */
  public function title(EntityInterface $node_preview = NULL): TranslatableMarkup {
    return t('Preview');
  }

This approach is broken after this change, because by the time $this->titleResolver->getTitle($request, $route_match->getRouteObject()) returns a result it already is a string. And the string primitive does not have a render() method.

What is the new way to accomplish overriding the name of an off-canvas dialog?

🇺🇸United States cosmicdreams Minneapolis/St. Paul

It this is what we want then we should also require that this module uses Drupal 10.3 instead of 10.1. If someone WAS using Drupal 10.1 then they would need to enable the experimental module.

🇺🇸United States cosmicdreams Minneapolis/St. Paul

There are a number of changes that result from committed work for Drupal 11 that we also need to reconcile for the module. All of that work will require us to dive into our javascript implementation quite a bit.

While doing that work we can see if we can code in support for this use case.

🇺🇸United States cosmicdreams Minneapolis/St. Paul

Interesting...
I'm not sure if there's anything that we're doing that would cause views to uninstall. I'll look deeper.

🇺🇸United States cosmicdreams Minneapolis/St. Paul

Ah yes, view_mode, that will be important. With the addition of that, the route is what I was thinking of too.

In addition to this, I also think views integration would be awesome and maybe not that hard to implement. If we have a working Renderer of HtmlFragments then we can just call it for the View.

So far our build list includes:

To build

A service that produces HtmlFragments of an entity. Arguments: entity_type, entity_id, view_mode
A controller that accepts our arguments and uses the service, delivers responses
A route that requires arguments of entity_type and entity_id. Optionally includes view_mode (default_value: 'default')

Future work

Reuse the service in a Views plugin so we can create a View that delivers an HTMX-compatible response.

🇺🇸United States cosmicdreams Minneapolis/St. Paul

This EventSubscriber seems to be responsible for what you're describing above:

<?php

declare(strict_types=1);

namespace Drupal\htmx\EventSubscriber;

use Drupal\Core\Render\PageDisplayVariantSelectionEvent;
use Drupal\Core\Render\RenderEvents;
use Symfony\Component\EventDispatcher\EventSubscriberInterface;

/**
 * Drupal should return a simple page for any route with our route option set.
 *
 * @code
 * route.name:
 *  ...
 *  options:
 *    _htmx_route: true
 * @endcode
 */
class HtmxPageDisplayVariantSubscriber implements EventSubscriberInterface {

  /**
   * Selects the simple page display variant.
   *
   * @param \Drupal\Core\Render\PageDisplayVariantSelectionEvent $event
   *   The event to process.
   */
  public function onSelectPageDisplayVariant(PageDisplayVariantSelectionEvent $event): void {
    $routeMatch = $event->getRouteMatch();
    $htmxRoute = $routeMatch->getRouteObject()->getOption('_htmx_route') ?? FALSE;
    if ($htmxRoute) {
      $event->setPluginId('simple_page');
    }
  }

  /**
   * {@inheritdoc}
   */
  public static function getSubscribedEvents(): array {
    // We want to run after BlockPageDisplayVariantSubscriber.
    $events[RenderEvents::SELECT_PAGE_DISPLAY_VARIANT][] = ['onSelectPageDisplayVariant', -100];
    return $events;
  }

}

The documentation calls out it's function by requiring developers who are aiming to use the route to include a route parameter to opt-into using this logic.

So this appears to be an underlying mechanism one could use to create your own routes that deliver HtmlFragments for entities.

What is the difference between this implementation and what I'm describing in Problem/Motivation section of the original issue description?

In a sub-module that can be optionally turned on, we can provide an example implementation of this service by providing a route that can render an {entity_type}/{entity_id}. That would be a common use case for the service. Perhaps an excessive one.

Better yet, we could find a way to integrate with Views in providing a way to return a result as an HtmlFragment.

🇺🇸United States cosmicdreams Minneapolis/St. Paul

Added Proposal: New HTML response service Active based on the discussion above.

🇺🇸United States cosmicdreams Minneapolis/St. Paul
🇺🇸United States cosmicdreams Minneapolis/St. Paul

cosmicdreams created an issue.

🇺🇸United States cosmicdreams Minneapolis/St. Paul

@brianperry also it might be cool to see a case study on how you make your slides with View Transitions. That would be a cool walkthrough.

🇺🇸United States cosmicdreams Minneapolis/St. Paul
🇺🇸United States cosmicdreams Minneapolis/St. Paul

For better or worse, at least for version 2.x, we've made the architectural decision to store the settings in localStorage.

We can revisit that decision later.

For the rest of your points, we can see what refinements we can make.

Please update the description on what you think the next steps are.

🇺🇸United States cosmicdreams Minneapolis/St. Paul
🇺🇸United States cosmicdreams Minneapolis/St. Paul

cosmicdreams created an issue.

🇺🇸United States cosmicdreams Minneapolis/St. Paul

Test comment

🇺🇸United States cosmicdreams Minneapolis/St. Paul
🇺🇸United States cosmicdreams Minneapolis/St. Paul

One likely solution is to use the new iframe element but I suspect there's something bigger wrong than that.

🇺🇸United States cosmicdreams Minneapolis/St. Paul

cosmicdreams created an issue.

🇺🇸United States cosmicdreams Minneapolis/St. Paul

It makes sense to provide the ability to control these settings from an administrative perspective. I think what will need to be proved in user testing is where users expect to find these settings and how the interact with the controls.

I think having a settings button in the preview pane is a good place to discover these settings.

🇺🇸United States cosmicdreams Minneapolis/St. Paul

It's intended to be used when the node_edit page is opened to determine if the preview should launch immediately, or when the user clicks the preview button.

🇺🇸United States cosmicdreams Minneapolis/St. Paul

I'll have to manually test this.

🇺🇸United States cosmicdreams Minneapolis/St. Paul

I think we got this change in another issue.

🇺🇸United States cosmicdreams Minneapolis/St. Paul

The way localSettings work is to provide settings locally to the device. A natural consequence to having localSettings is that the device will use the localSettings before the remote settings (in this case the server provided defaults).

If we can accept this architectural limitation then we can continue to have per-device settings. If this is a deal breaker then I'll have to make this consequence clear in the documentation.

🇺🇸United States cosmicdreams Minneapolis/St. Paul

It shouldn't be the case that you need to enable Same Page Preview for each content type separately. The content type settings are intended to be an opt-out strategy.

🇺🇸United States cosmicdreams Minneapolis/St. Paul

What the "On by default" is intended to control is the ability to open the Same Page Preview iframe whenever content loads. The "preview" is "on" by default instead of being "on" whenever you click the preview button.

For users (while using their device) would be overwhelmed by this, keeping this feature is an important consideration.

Is your issue the nameing of the Setting or it's logical organization?

🇺🇸United States cosmicdreams Minneapolis/St. Paul

I think I understand what you're asking for. You want the administrator to be able to modify the default values for each of these settings. Is tha right?

In that context, I imagine this scenario:

  1. A content administrator logs into the site and uses same page preview. At this point localSettings stores a value for each setting (whether it is activated)
  2. A site administrator modifies what the default values for these settings
  3. The content administrator doesn't receive this change because the local values will override the default value.

Or rather, the default values only impact users that have never used+devices same Page Preview or have flushed out the localSettings.

🇺🇸United States cosmicdreams Minneapolis/St. Paul

@DieterHolvoet MR created please review.

🇺🇸United States cosmicdreams Minneapolis/St. Paul

Agreed. I'll add that into the documentation. I would consider an overhaul of the documentation but for now I'll just add a one-liner.

🇺🇸United States cosmicdreams Minneapolis/St. Paul

@bsnodgrass That's an administrative setting

🇺🇸United States cosmicdreams Minneapolis/St. Paul

Good news! I'll be traveling to Midcamp tomorrow. I'll be meeting with previous contributors of this module and talking up a gameplan for work (leading up to Drupalcon).

I'll put this to top of my list of issues to consider. Stay tuned.

Production build 0.69.0 2024