This feature should be backported to 2.0.x.
I have an idea on how to do this.
Project Browser, when applying a recipe, should check for a redirect
directive in the recipe's extra
section. If it's there, it's a YAML-encoded Url
object (something we already support, for follow-up tasks). That's the URL you get redirected to. Simple.
But there's one restriction: if you apply multiple recipes at once, all redirects are ignored. You can only be redirected if you apply a single recipe.
Postponing on 💬 The Cropper library referenced in the README and in the libraries info is deprecated Needs work .
One question here about whether we're breaking backwards compatibility.
Gin released 5.0.4, so this is probably close-able.
phenaproxima → made their first commit to this issue’s fork.
Why was views.view.scheduler_scheduled_content.yml
deleted?
Reverted because this broke the site template under development in Drupal CMS. :(
Reverted because this broke the site template under development in Drupal CMS. :(
Shipped in 2.x with extreme prejudice.
@pameela, are you sure this is correct? AFAICT, this is adding alignment to one format, but not both. No objections to merging it, as long as the difference is legit.
In the meantime we are also using simpleConfigUpdate and including the full set of matchers.
Not true anymore :) We are in fact polyfilling the action by synthesizing the full config action definition. It's a neat trick: https://git.drupalcode.org/project/drupal_cms/-/merge_requests/644/diffs...
But it would be better to have the real McCoy committed and released for us to rely on instead!
Not a blocker for Drupal CMS, but why not also turn setMatcherConfig()
into an action while we're at it? That could surely be useful too.
We can't do this in drupal_cms_content_type_base
, because we're introducing an implicit (and unnecessary) dependency on Canvas.
This should be done via a config action in drupal_cms_starter
.
Re-titled to remove DrupalCon mention, and merged into 2.x.
If there's a follow-up to be done, it's probably in Drupal CMS. We can mark this issue fixed.
No objections to that fix. Needs a test though!
Fair! I know what I personally would like the most, but I have no particular dog in this fight as long as it gets done somehow.
To me, #3 is the most straightforward option. If we mess with the file cache, we're inviting ourselves to enter a world of pain. I also think the third option is the most "obvious" one, and therefore best for DX. I also like that it allows different plugin types to decide if they want to support additional properties or not.
I bet we could reduce implementation pain if we added a trait for plugin attributes that support additional properties.
The export is currently implemented by Drupal CMS Helper, not Canvas itself, and Drupal CMS helper can make this improvement.
phenaproxima → made their first commit to this issue’s fork.
I've renamed the repository to https://github.com/phenaproxima/canvas-demo and updated Packagist to update automatically from there.
The old xb-demo URL redirects to canvas-demo -- thank you, GitHub!
I have not renamed the actual Composer package, but could get around to that if we really care about it. We'd need to keep xb-demo as an alias, I think, but Composer should support that with the provide
or replace
sections of composer.json, if I'm reading the relevant documentation correctly.
phenaproxima → made their first commit to this issue’s fork.
After some discussion with @alexpott, I decided to revert this and do a quick hail-Mary patch of replacing all occurrences of experience_builder
with canvas
.
To provide a little more context: these components should never have been committed to the 1.x branch of Drupal CMS Olivero, but if I remember correctly, they landed before Drupal CMS 2.x was branched. So they kind of snuck in here, and are being used in certain corners of this theme (e.g., node--card.html.twig
is using the card
component).
So, realistically, removing them outright is not an option. But it is equally unacceptable for this theme to break Canvas.
There is probably more work to be done to make this theme play nicely with Canvas, even if it never fully embraces it. At the very least, we need a GitLab CI workflow which confirms that Canvas can be installed alongside this theme. I don't have the bandwidth for that right now, but I'll open a follow-up. Hopefully, the simple replacement I mentioned above will be enough to get past the immediate WSOD.
I'm not sure on how it could have worked in the first place, if you were using components that reference schema objects from experience_builder
-- which is now defunct?
@rodrigoaguilera, yes, a module could extend this by implementing hook_config_schema_alter() to change the options available to the validation constraint, then probably a targeted form alter to put additional options into the settings form.
Let's switch 2.x HEAD to Gin 5.x-dev, then!
After some discussion with @catch, I decided to add the link relationship as an additional configuration key, because trying to convert link_to_entity
into a nullable string would put us into what can only be described as the 10th circle of update path hell.
But adding a new config key is straightforward, so I think this is ready for review.
Backport MR opened; since this changes behavior of an existing API, should we maybe check with the release managers that this is backport-safe?
Yeah, this is a duplicate. Closing tentatively for now. Be sure to update Drupal CMS Olivero to 1.2.4 or later before trying again, but feel free to reopen this if updating doesn't solve the problem for you.
I agree that'd be cleaner from an implementation standpoint, but I'd like @pameeela's sign-off on that.
I was able to reproduce this problem, and I confirmed that it disappears with this bug fix. I'm gonna merge it and roll a new release, since this can interfere with people testing Canvas innocently on Drupal CMS 1.x.
It's possible you're trying to use an older version of drupal_cms_starter that no longer matches what Project Browser expects.
Try updating drupal_cms_starter: composer update --with-all-dependencies drupal/drupal_cms_starter
If Composer complains that it's not installed, re-require it: composer require --update-with-all-dependencies drupal/drupal_cms_starter
.
Does any of that help?
This should be real easy. All we need to do is add this in drupal_cms_installer_theme_form_installer_site_template_form_alter()
:
$form['add_ons']['#required'] = TRUE;
Tagging as a novice issue.
Yes! \Drupal\Core\Config\ConfigManagerInterface::getEntityTypeIdByName()
will do what you want.
Thanks for the bug report! I assume you were trying to spin up Drupal CMS 2.x to play with Canvas? (I don't know how this error could occur in Drupal CMS 1.x.)
Here's the thing -- Drupal CMS 2.x is still under very heavy development and is not yet stable or usable. Some of our dependencies are not tagged yet, and are changing on a daily, or even hourly, basis. These sorts of bugs happen all the time and we're fixing them as they come up. :)
So I'm gonna close this one out, I think, since it's not something we're liable to be able to reproduce reliably as our dependencies change. If you want to try Canvas, the best option right now is https://github.com/phenaproxima/xb-demo.
We can override the select.html.twig
template for this particular select list and style it however we want. This is probably not too tricky at all, honestly.
@jurgenhaas showed me how to do this with ECA, but I think it would make more sense to do this when ✨ The config_save action should be able to treat its value as a partial config entity Active is available in a tagged ECA release.
OK, I deleted every component that mentions XB in its metadata. I left the components which don't, since they're not harming anything.
Can you try this branch out to see if it still breaks in Canvas?
The 2.x branch of this theme is unmaintained, and the 1.x branch is only minimally maintained. We don’t plan for this theme to ever fully support Canvas.
Having said that, it should not be causing a WSOD if you install Canvas.
So the solution here is probably to remove the components we added to it.
Got it to work. It's a bit messy but it works. We can clean it up later.
One nit but otherwise I have no complaints here.
I could do form-element--site-template
, would that work?
Then let's remove it, rather than dealing with the rigmarole of making sure it doesn't show up Canvas.
phenaproxima → made their first commit to this issue’s fork.
phenaproxima → made their first commit to this issue’s fork.
Merged into 2.x. Thanks for doing this!
We actually already have a template suggestion for those things specifically! I think it's input--radio--site-template
, based on input--radio
.
Reviewed, and looks better and better -- I think this will be even better if we use a generator function to yield the entities to export, rather than trying to load them all at once and do an array_chunk() on them. There is also more verbosity than is generally needed, and some changes which don't add anything to the patch and should be reverted. But it's shaping up!
Damn you are fast. Assigning to myself to review.
That makes sense. In any event, feel free to alter the markup if you need to, as long as the overall functionality remains intact. :) If this is ready, go ahead and kick it to needs review and I'll make sure @pameeela has a look at it.
This seems like a great start -- and bless you for writing tests!!! -- but it's a little hard to read and understand, overall, because of the complexity. I think we might want to scale back a few things, at least for now:
- The progress bar is a nice touch, but it adds weight to this MR and would be best done cleanly in a follow-up.
- Let's not have the ability to just export all content, regardless of entity type. Such a function merits additional consideration and we probably would want to think it through in another issue. So, for now, let's require an entity type ID at the very least.
For the record...if it will help style things as per the design, it is totally okay to convert that from a select list to something else. As long as changing the language redirects to the same URL with a different langcode
query parameter, it doesn't matter whether it's a select or not.
Ah! Okay, that gives me the context I would need to fix it.
We could remove it, but that would wipe out quite a bit of the value of the view for Drupal CMS (to the point where we'd probably need to ship our own override of the view anyway). We are already using the block display, because we put a list of recent pages on the admin dashboard.
Explanation of current PHPUnit failures:
Drupal\Tests\canvas\Functional\ApiUsageControllersTest
: Not sure what's happening here; a component was previously mentioned in an array and now it's...not? I don't know how adding a view could have caused this, but it feels like it might be exposing some other, preexisting bug, since the presence of a view can trigger many different code paths that were previously not exercised.\Drupal\Tests\canvas_ai\Kernel\Plugin\AiFunctionCall\SetAIGeneratedComponentStructureTest
: It passes locally for me, but was failing on CI. I have no idea how adding a view could have affected this either way. I decided to try to flail-fix it by having it install the canvas_page entity schema.
Paired on this with @galactus86, so it has his sign-off. Merged into 1.x.
How? There doesn't appear to be a route for creating a new Canvas page at all. The Page entity type doesn't define an add-form
link template. Not seeing any such route in the routing.yml.
Am I missing something?
Setting back to NR for now, as the test failures appear to be unrelated.
Okay, new idea here -- let's create two new ECA models:
- One which, whenever a content type is created, adds the SEO fields to it (and its form display). This will replace some of the config actions in the recipe.yml currently.
- One which, when the drupal_cms_seo_tools recipe is applied, executes that workflow (triggers the event, I suppose) for each extant content type.
Would that work?
This issue is hard to finish.
We need to rethink the approach, because it's fundamentally incompatible with recipe unpacking. If you unpack the drupal_cms_seo_tools recipe (which you should), it is no longer visible to Composer -- and therefore, by extension, unavailable to ECA.
That works as intended - merged into 2.x!