I think Canvas developers should buy drinks for everyone who has been putting up with all the rough edges and providing invaluable feedback. π
Also, #11: Boooo! (Just kidding. π)
I think this is a great idea! The Tailwind Plus templates made by the Tailwind team itself do the same: The different lines are always logical groups that make sense in the context of the styling being done. To automatically sort the class names within the lines (see the recommended sorting), we could use the Prettier Plugin for Twig along with the official Prettier plugin for Tailwind CSS.
balintbrews β created an issue.
We've made UI changes since this was reported.
I haven't seen this issue for a long time. @lauriii β have you seen it by any chance?
β¨ Include navigation events in undo/redo Active made the problem less severe.
Does this mean we can/should remove updateExistingComponentValues? We also have _updateExistingComponentValuesForLinking, which is doing similar workarounds. Should we account for these in this issue, or would that be more appropriate to tackle later?
I don't see updateExistingComponentValues being used anywhere in the codebase. I know it was used before by Canvas AI UI code, but looks like it's gone. _updateExistingComponentValuesForLinking is being used by the content templates functionality, so maybe we should wait for
π
Add e2e test for `ContentTemplates` feature
Active
to land, then attempt to refactor?
As we're picking this up again, I would like to explore doing the preloading explicitly in the UI app, so we can avoid having to make backend changes when we update tailwindcss-in-browser in the UI codebase, which could bring in the Lightning CSS WASM file with a new version number appended to the filename. (I understand there is a kernel test added to verify that, but since this is an update in the UI package, I would prefer if we dealt with it and verified it in the UI codebase. Otherwise we'll also need to make sure to run kernel tests on CI when the package-lock.json file changes.)
The default export from both lightningcss-wasm and @swc/wasm-web is a function to initialize the WASM file. We can call those functions in the UI app high in the component tree, and even set a state to present different UI elements before they're ready when the code editor is opened, and most importantly, prevent running compilation for a code component before the WASM files are available. We wouldn't have a way to know when that is without using the init calls.
Here is how to test this change.
To apply the change locally, run npm install and npm run build in the ui folder, then make sure to do a hard refresh in your browser.
Create the following Code Component:
export default function() {
return (
<p className="hidden md:block">
I should be visible on medium-sized screens.
</p>
);
};
- Test in an environment where the change by this issue is not applied. Notice that the paragraph is never visible.
- With the change by this issue, the paragraph needs to become visible on medium-sized screens.
balintbrews β created an issue.
These issues occurred in a certain environment with Canvas.
Background color class is being overridden by another CSS rule
This was caused by a theme including the core/normalize library. Removing that solved the issue. I'm thinking about how we may want to detect from Canvas if that library is used, and suggest actions for the user, but I'm not ready for a proposal and a well-written issue for this.
Tailwind classes like hidden md:block aren't working as expected once the component is placed on the screen.
This was caused by hidden.module.css from the core System module. Doing a library override to move the .hidden class into a layer helped. Here is a new idea how to solve that. I'll create the d.o. issue for bringing in the new tailwindcss-in-browser version if we decide to go ahead with the solution.
Dynamic components like Main Navigation (component), Navigation Menu (Demo component) aren't rendering as expected on a page
Again, specific to that environment, I described the problem and the solution in this issue for a longer term fix: π Evaluate support for preact/hooks specifier in Code Component import statemements Active .
balintbrews β created an issue.
After digging into this, and also verifying it with a project using regular Tailwind CSS via Vite, I realized that this is the expected behavior. It was not the behavior in Canvas previously due to us missing the CSS layers in our output, which was fixed in β¨ Modifying Tailwind theme in global CSS Active a few weeks ago.
The solution is to move those declarations in the global CSS inside the base layer. For example:
@layer base {
a {
font-weight: 700;
}
p {
font-weight: 400;
}
}
balintbrews β made their first commit to this issueβs fork.
This needs to be fixed in src/features/code-editor/component-data/Props.tsx, and tested in tests/unit/code-editor-component-data-props.cy.jsx.
#6: I opened π Audit Code Component HTTP endpoints to allow omitting properties Active for that. Let's merge what we have here, since it's already a solution with tests, and the patch were successfully aiding all our DrupalCon Vienna demos last week. π
balintbrews β created an issue.
Linking to the related core issue for visibility. In the meantime, since we're already doing our own import map anyway, we should allow this alteration. Once there is a core API, we can revisit this.
balintbrews β created an issue.
Let's postpone this on #3552788: Automate the release process β .
We discussed this with @justafish and agreed:
- Committing the release artifacts should happen as a detached commit. The release tag Drupal.org will pick up will get added to this commit.
- We'll need to add
npm version $TAG --allow-same-version --no-git-tag-versionfrom the original script. - If it makes sense here, we could address π Unable to add Drupal Canvas UI to GIT repository due to .gitignore rule Active , but it's also fine to tackle that after this issue lands.
Here is what's happening. Our logic that calculates data dependencies based on a Code Component's JavaScript source code incorrectly assumes that if anything is imported from @/lib/drupal-utils, we'll have a data dependency for something in drupalSettings, and initiates it as an empty array. For getPageData() and getSiteData() from @/lib/drupal-utils this is correct, and that array will be filled with the correct dependencies. However, @/lib/drupal-utils also provides sortMenu(), which is purely a utility function with no data dependencies, so the array remains empty if nothing else is imported that would warrant a data dependency.
So this import in the attached Code Component will produce the empty array:
import { sortMenu } from '@/lib/drupal-utils';
In my opinion, the backend is way too rigid to error out because of that, but that's a different discussion, and is a broader one than the scope of this issue. So I'm solving this from the frontend codebase, where the easiest thing to do without breaking changes is to make sure that we never send an empty drupalSettings array in dataDependencies.
Workaround until the fix lands:
import { sortMenu, getSiteData } from '@/lib/drupal-utils';
Just importing that additional package will add something in dataDependencies.drupalSettings, and will avoid the error.
The initial implementation landed in β¨ Extension plugins, UI, and browser API for reading component tree Active .
We now have @drupal-canvas/extensions!
I tested manually to make sure the test failures are not showing a real problem.
balintbrews β created an issue.
The way to validate this would be running ESLint, and using the no-custom-classname rule from eslint-plugin-tailwindcss. So it's not easy to add until we figure out a way how to make it happen that we can run ESLint and other JavaScript-based code to check the output.
I realized another consequence of this issue in relation to working with Code Components outside of the in-browser code editor, using the CLI tool. I updated the issue summary to reflect the current reality.
balintbrews β created an issue.
The plan I outlined in the ADR was already approved by @lauriii. He and I worked together on the metadata schema, so that's also validated from a product standpoint.
balintbrews β changed the visibility of the branch 3514033-adr--extensions-api to active.
balintbrews β changed the visibility of the branch 3514033-adr--extensions-api to hidden.
balintbrews β changed the visibility of the branch 3514033-adr--extensions-api to hidden.
balintbrews β changed the visibility of the branch 3514033-adr--extensions-api to hidden.
balintbrews β changed the visibility of the branch 3514033-adr--extensions-api to hidden.
This happened in π Move code component starter template documentation from inline comments to online docs Active .
balintbrews β created an issue.
It seems that this is NOT introduced by
β¨
Include navigation events in undo/redo
Active
. I was able to reproduce with a version earlier than that, and with React.StrictMode disabled. The changes done in
β¨
Include navigation events in undo/redo
Active
also make it visible in React.StrictMode.
So this might be tricky. When we add a component to the editor frame, the instance receives a unique ID, right? If we undo that, then redo, we'll end up with a new UUID. So trying to get back to the previously assigned UUID in the URL wouldn't make sense, because that ID is not valid anymore.
#2: Whoever is working on the fix, they will need to look into this anyway, so I see no reason for postponing with that status. We may want to postpone this, however, for different reasons. π
I think we should get π Redo action does not update the URL Active right first, and then tackle this. There may not be anything to tackle at that point. Though it's interesting why this happens only with that one component based on Mayur's testing.
balintbrews β created an issue.
Playwright test failure on this MR matches an earlier failure from one of the recent commits in 1.x.
Ready for review with approach #2 implemented in merge request !141. Heavily unit-tested, including the hook that syncs the route information between the Redux store and when navigating with React Router. I'll add a simple end-to-end test in β¨ Include code editor in undo/redo Postponed .
balintbrews β created an issue.
balintbrews β created an issue.
balintbrews β created an issue.
While we're here, can we please make this change? It would be the last thing to make the library look polished.
diff --git a/src/Plugin/Canvas/ComponentSource/JsComponent.php b/src/Plugin/Canvas/ComponentSource/JsComponent.php
index 75fc6432..63cf5ee2 100644
--- a/src/Plugin/Canvas/ComponentSource/JsComponent.php
+++ b/src/Plugin/Canvas/ComponentSource/JsComponent.php
@@ -291,7 +291,7 @@ final class JsComponent extends GeneratedFieldExplicitInputUxComponentSourceBase
'id' => self::SOURCE_PLUGIN_ID . '.' . $js_component->id(),
'label' => $js_component->label(),
// @todo Update in https://www.drupal.org/project/canvas/issues/3541364. This causes a "@todo" Folder to be created.
- 'category' => '@todo',
+ 'category' => '',
'provider' => NULL,
'source' => self::SOURCE_PLUGIN_ID,
'source_local_id' => $js_component->id(),
I realize it's not related to this issue, but it helps reducing the clutter we don't need anymore, and we have a @todo comment anyways to do this properly.
π Hide "Add new folder" button under a feature flag, move "Add new code component" button to Manage library Active took care of this.
@omkar-pd, thanks for your previous work on this, and sorry that we missed reviewing and moving it along. I credited you on the other issue.
Yet another massive @jessabaker-style refactor and UX improvement! π Thank you for going above and beyond.
I quickly added the debounce asked by @hooroomoo. While testing, I discovered π Hovering library items sometimes opens preview thumbnail with wrong size Active , opened as a follow-up so we don't block the huge changeset and all the UX improvements this MR brings.
balintbrews β created an issue.
Works beautifully! And the fact that you took care of the CLI tool, and made things work there, too, is very impressive. π Thank you for another great contribution!
Sorry, I had the issue open from earlier when I updated the status.