@doxigo

Thanks, merged.

thanks, merged

Okay I pushed a backward compatibility fix for this since one of my projects also got hit with this. marking this as fixed for now, and taggign a stable release 6.0.0 🎉

Perfect Dan, I wanted to merge it but seems like MR is in draft

Hey Dan,

A couple of notes on my end regarding the issue:

> Currently when a radix template changes, existing sites will not receive the change without manually copying the change from the starterkit to their sub-theme.

That's by design, and I believe that's okay to be like this, any older website that is already built, shouldn't be needing to change their markup (eg. templates and so on), so what they currently have should suffice. If they are keen on being updated, they should manually review and update if needed.

> When the change is accompanied by a corresponding change to the component, this can cause incompatibility where none would otherwise occur.

This is the tricky part, but we've been doing this since 8.x.4.x more or less, we just need to be more mindful of backward compatibility when changing the components.

> Also, core discovers the templates in the starterkit and uses them in the absence of a template in the sub-theme.

That's the core problem to be addressed, when we set the starterkit: true it should just ignore the templates, but for now let me take a look, I might have some solutions for this, eg. fully remove the starterkit once it gets copied

Even though this is okay to add the issues here, if anything we should add the related issues to Github repo: https://github.com/doxigo/drupal-radix-cli

That aside, glad your issue is fixed once you updated your node path, I can look into this further see if it's something we can help with or not.

Okay we will definitely will have unhappy users, but what's done is done. I suppose we can keep the 6.0.x since in Drupal, 6.1 and 6.0 is not that different. Also we are still on RC, so we have an excuse :)

to add my two cents:
If we make a change that has breaks things for existing websites, we should do a revert for now. if it's a major change that we can't live without, we can do a new major release even and move forward with that. Otherwise not all people will find this thread.

Hey James, thanks for a thorough testing, in the end if watching the build does the job, go for it. In my head, we needed to watch the source rather than the build but if it fails as you suggest, well... it fails.

There's no harm in switching the browserSync watch in general.

@jamesoakley Yes to all. any SCSS/non-build JS files (the ones that start with a _) should be watched. could you do a MR then please?

Hey guys, I'd say that could possibly make sense in some scenarios but not generally no. but there might be some downsides:
- Watching build files could cause recursive reloading
- Build files are outputs, not sources
- Changes to build files are already handled by laravel-mix

in general we can do a bit of testing and see how it turns out to be but BrowserSync should only watch source files

Hey guys,

Dan as I mentioned in the email:

I believe this could be a breaking change and break every website that there is if it’s in the core Radix. That aside the I can’t recall on top of my head about the error that this causes in multilingual pages, and the issue but I know that I’ve used the path alias based suggestions quite a lot.

If it were up to me, I’d try to fix it rather removing it, but feel free to make your own decision.

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

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

Perfect, thanks

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

lovely, thanks. merged

Thanks, merged

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

Thanks, merged

Seems to be some conflicts post merging other MRs

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

Thanks, merged

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

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

Could you please provide a MR?

Thanks for the MR, merged

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

@erier even though that does work with Autodocs but the "Show code" part still does nothing. Am I missing something? does that work for you?

I agree, we should get rid of {% apply spaceless %}
I would really appreciate a MR if possible

Thanks a lot for the MR Nicholas, merged

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

Thanks a lot ckng, merged.
Pierre I'll check and see what we can do.

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

Closing, as it is fixed in Gin

With tags: ['autodocs'], I noticed that the "Show code" also doesn't really load anything, I might be missing something

Must've missed your point regarding the first forwarding slash, thanks. fixed

What exactly the Upgrade status says? is it your Sub-theme or Radix theme itself?

I'm marking this as closed, if anyone wants to help out with a new documentation that we can share for Radix 6.x I'd be glad to add it to our official docs. for now marking this as done

I personally couldn't get localhost:3000 to load with the approach above, but for now I created this page:
https://radix.trydrupal.com/radix/installation/setup-with-ddev

Please do a read and suggest any improvements you have in mind

Marking this as postponed for now since it seems to be a core issue, will open up an issue in core

Thanks guys, updated on the latest dev

Hey Alan, I updated the biome.json config on the latest dev (Check it out here), it is now more clear and easier to adjust, marking this as done, thanks a lot

In patch #23, you cannot uncheck the merged items once you select them. #20 works as expected

Thanks for sharing the solution and the great blog post, I myself haven't gotten around the ddev approach yet, I'll try to add this to the documentation as well

Thanks guys for rather a quick fix on this one

Damn Drupal, yeah alright but I if that's the case we need to follow Drupal's standard on linting and so on, but again a preference I'd say but you are right we can keep it as the same Drupal

Based on your screenshot I suspect the issue is due to:
1. Whether a field is optional or required, eg. accepting an empty string so it marks as validated
2. If it's filled with text or not, hence the relative size of the icon.

Could you test this further and see if it's an actual issue or not Thomas? But I suspect this is a Bootstrap issue rather than Radix if it's not as ideal as it should be

Thanks Aleix for the guide, I'll mark this as complete but we can possibly move it to the documentation but for that we should probably avoid any CSS part and focus on more Twig/components side of things that are Radix specific.

Seems to be sth that needs to be addressed in core, will investigate further later on

This turned to be quite hard to fix, I'll spent some more time if possible soon but I suspect Layout builder in core is getting deprecated, not sure if it's wise to spend time on this

I can't recreate this and it works as expected. Maybe something has changed since you've created your Sub-theme.

I'd say the /web/themes/custom/ is the default custom path that people get when they setup a new Drupal website and it is good for new comers to know where things are

marking this as closed for now
Thanks

Pushed an update Milos, thanks for bringing this up

Hey Keegan and lelkneralfaro, I couldn't recreate the issue but I suspect the issue could be due to most of the cells in a component accepts type: array

Could you maybe help out by removing each part and test again to pinpoint the issue or update me on how to proceed to recreate the issue, thanks

I'd say this is a preference Alan, you can always configure the Biome extension on your IDE to convert things into space instead of a tab. Marking this as closed for now but thanks

Thanks a lot guys, updated.

Thanks for the patch and the issue, fixed

Also updated the docs, thanks

Thanks for the issue and the MR Ion, merged.

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

There's a page in the official documentation about that under "Migration and upgrading"

Even though that page does exist, the job is not straight-forward and really depends on the complexity of your 5.x theme and how well it was built considering components.

Marking this as closed for now, feel free to ask questions if you encountered any along the way.

@qusai taha, you can just click on the plain diff and that's a patch already, no need for a separate patch

definitely, I would appreciate it, thanks

doxigo → created an issue.

I'd say this is rather a blocker for any big organization to ever consider to use Workspace, big organizations constantly update their live instance, so this should be available only as an opt-in option.

I suspect we should update the init, just noticed that we are not adding the root

not sure if we can decide for each project what's needed or not. The idea is to ship with the features, people can opt-out and comment out what they don't need.

The ideal way would be to use @use instead of @import for sure but Bootstrap is not yet supporting that, hopefully in bootstrap 6.x

on the other hand _init.scss contains the following:

@import "~bootstrap/scss/functions";

// Custom overrides
@import "base/variables";

// Required
@import "~bootstrap/scss/variables";
@import "~bootstrap/scss/variables-dark";
@import "~bootstrap/scss/maps";
@import "~bootstrap/scss/utilities";
@import "~bootstrap/scss/mixins";
@import "base/mixins";
@import "base/utilities";

// Optional if using rfs()
@import "~bootstrap/scss/vendor/rfs";

So that's not with _elements.scss or _typography.scss

The main reason we are including init, is to make sure we have access to our variables, mixins and so on. It might not be ideal due to lack of @use but it can be just fine as is for now, you can always remove that manually.

Unless you have a better suggestion, I think it is safe to keep it as is.

Lovely, as always thanks Miloš

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

doxigo → created an issue. See original summary → .

It does make sense but not that much of an urgent issue that requires an action right away, props drilling isn't ideal but still gets you going a lot of times and it works fine as is in Radix, we can reconsider this at a later stage if needed, marking this as closed but will have it in mind if it comes to 6.0.x or the Tailwind version of Radix, thanks

Thanks Thomas, merged this one as well, not sure if this is the best solution but lets go with it for now

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

Looks solid Thomas, great work. I had to cherry-pick and UI wise I couldn't open a MR as well. Anyways, thanks a lot, merged.

I went on and updated the MR, thanks a lot, will check this a bit further

Hey thanks a lot for the work. Can you check the MR again and fix the merge issues?

Hey thanks a lot for the work. Can you check the MR again and fix the merge issues?

Can we get this fixed on 2.x and 3.x? seems rather high priority IMO

The MR works as expected, changing the status to RTBC

I placed this under the "Support request" but this doesn't seem to be Radix specific issue, you can render a new region like any other themes the way that you described. We might need more info to be able to help you but I suspect it might be a template not being read correctly, eg. you might still referencing the Base theme rather than your Sub-theme.

That aside try reading the Radix documentation thoroughly, that might help.

Closing the ticket for now but feel free to comment

Thanks @ckng for the comment, to address some of it:

- True that the usage of a core modal are low, but it would be ideal to make sure it still look the way it should in a Bootstrap env.
- The same for off canvas but within layout builder ecosystem it's constantly being used.
- Yes good idea to move it to the base theme
- Core always breaks the themes! overriding it actually prevents the possible future breakage more compared to extending it. That aside I don't believe you can extend it.
- Technically it's currently blocked since the override works for the off_canvas or for the modal, otherwise it breaks the other. for now it's commented out until we manage to fix it

Thanks for the patch, applied to the latest dev.

Thanks, merged

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

Thanks, this is merged.

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

oh my bad, was a patch there that made me believe so

doxigo → created an issue.