Belgium 🇧🇪
Account created on 7 January 2016, about 9 years ago
#

Merge Requests

More

Recent comments

🇧🇪Belgium BramDriesen Belgium 🇧🇪

bramdriesen created an issue.

🇧🇪Belgium BramDriesen Belgium 🇧🇪

I think this is ready for review once more. Preferably also some manual testing as there was one issue the tests didn't catch which I'm still not sure why that test was nog failing (see last commit).

🇧🇪Belgium BramDriesen Belgium 🇧🇪

fixed a few things.

🇧🇪Belgium BramDriesen Belgium 🇧🇪

one comment was not fixed.

🇧🇪Belgium BramDriesen Belgium 🇧🇪

+1 For RTBC. Totally forgot about this issue.

🇧🇪Belgium BramDriesen Belgium 🇧🇪

Thanks for testing!

🇧🇪Belgium BramDriesen Belgium 🇧🇪

Probably needs a rebase as well before re-attempting this as some modules are removed.

🇧🇪Belgium BramDriesen Belgium 🇧🇪

Looks like the failing tests were false positives.

🇧🇪Belgium BramDriesen Belgium 🇧🇪

We can't just change this with the current compatibility scheme. This will only work going up from 10.3 and will break everything below I think.

@ggh I noticed your effort on fixing those things for a lot of modules. Note that this falls under the credit abuse policy as this is an task that can be automated. https://www.drupal.org/drupalorg/docs/marketplace/abuse-of-the-contribut...

🇧🇪Belgium BramDriesen Belgium 🇧🇪

I did go ahead and replace the mild grey for non-stable releases with the mildest grey. This should improve contrast, and reduce the highlighting of prereleases.

That looks a lot more easing on the eyes, and from a quick glance, looks a lot different to the blue so that's already a lot better.

Whenever we do small design changes, we’ll be incorporating the new branding elements. Hopefully things don’t look too mismatched as they gradually change, but it's a lot more doable than attempting to get everything right all at once.

Yes 100% agree, small incremental changes are easier to manage instead of a big bang. And let's not forget with designs, there is not a single design that works for everyone 😊.

Thanks for this small, but great change @drumm!

🇧🇪Belgium BramDriesen Belgium 🇧🇪

I do think there should be clear a distinction between covered and not covered by the security advisory in the releases section. Besides the little shield, it should be very clear like it used to be.

Really not convinced blue and grey are the best colour choices. Also, with the goal to modernise, this feels like a step back with the rounded corers, it really doesn't seem to fit here. I can understand this for marketing content, but not here :-).

🇧🇪Belgium BramDriesen Belgium 🇧🇪

I don't like the colour change. I don't really have an issue with the blue one for stable & covered releases. But the grey is really not as clear that it's different for an none stable release or not covered project. Yellow/Orange pops more as "hey watch out". Grey is too neutral for me, and I think this will cause confusion. When landing on a project with no stable/covered release, it's really hard to tell without looking for the shield or the tagged version.

I guess this was done because the --drupal-yellow from the brandbook is too "hard". But I really feel we should pick a different collour as grey which is close to the blue.

Slack thread: https://drupal.slack.com/archives/C1BMUQ9U6/p1742889668136919

🇧🇪Belgium BramDriesen Belgium 🇧🇪

Let's use official sources instead of Wikipedia et al (see IS).

Pronounced “droo-puhl,” the name derives from the English pronunciation of the Dutch word “druppel,” which means “drop.”

Source: https://www.drupal.org/about/history

🇧🇪Belgium BramDriesen Belgium 🇧🇪

Check if possible to map entity type as string and the values (checkboxes) as boolean.

🇧🇪Belgium BramDriesen Belgium 🇧🇪

Ah no, just checked what I did here and this is not finished

🇧🇪Belgium BramDriesen Belgium 🇧🇪

I guess we can set it as RTBC then

🇧🇪Belgium BramDriesen Belgium 🇧🇪

I think this looks okay now. Not a native English speaker so might have some grammar issues.

🇧🇪Belgium BramDriesen Belgium 🇧🇪

bramdriesen created an issue.

🇧🇪Belgium BramDriesen Belgium 🇧🇪

bramdriesen created an issue. See original summary .

🇧🇪Belgium BramDriesen Belgium 🇧🇪

I think there are too many changes being made here. I think we only need to inject another dependency into the base class.

🇧🇪Belgium BramDriesen Belgium 🇧🇪
🇧🇪Belgium BramDriesen Belgium 🇧🇪
🇧🇪Belgium BramDriesen Belgium 🇧🇪
🇧🇪Belgium BramDriesen Belgium 🇧🇪
🇧🇪Belgium BramDriesen Belgium 🇧🇪

Wrong Frederik :)

🇧🇪Belgium BramDriesen Belgium 🇧🇪

I personally have no issue with pushing one solution. When observing slack and other channels where people new to Drupal ask for a local set-up, 9/10 I see DDEV as the first answer and what they decide to try.

I guess there is no harm in still listing other solutions like Lando and others. But with the idea of streamlining the documentation pages and to maintain those, one solution would be best.

🇧🇪Belgium BramDriesen Belgium 🇧🇪

Good question, I didn't even know this route existed. I don't really see an issue with this being configurable through (yet another) checkbox though. Since this is a custom route being defined by the keycloak module there is no off the shelf module that allows to block it.

Changing to feature request.

🇧🇪Belgium BramDriesen Belgium 🇧🇪

You can use role mapping to do role assignment, but you currently can't prevent them from authenticating and getting a permission-less account this way.

🇧🇪Belgium BramDriesen Belgium 🇧🇪

There are test failures. Looks like the forgot password message is not being shown?

Behat\Mink\Exception\ResponseTextException: The text "Unrecognized username or password. Forgot your password?" was not found anywhere in the text of the current page.

🇧🇪Belgium BramDriesen Belgium 🇧🇪

bramdriesen made their first commit to this issue’s fork.

🇧🇪Belgium BramDriesen Belgium 🇧🇪

Patch looks good, but easier for the maintainer if it's a MR.

🇧🇪Belgium BramDriesen Belgium 🇧🇪

Hi @lavanyatalwar

First of all thanks foor looking in depth, and not doing something blindly ;-).

I'm aware that hook_user_login() is happening after the actual login. I guess it's possible to do an forced logout from that point somehow.

🇧🇪Belgium BramDriesen Belgium 🇧🇪

Thanks!

🇧🇪Belgium BramDriesen Belgium 🇧🇪

I'm in the process of obtaining an API key since they changed their policy. Would it be possible to become co-maintainer so I can start working on this in my spare time? :-)

🇧🇪Belgium BramDriesen Belgium 🇧🇪

Is this duplicated by 🐛 There is no way to make a button element that can use the AJAX functionality Needs review ? It's a bit of a different approach judging from the code changes. Will need a reroll anyway because there is a merge conflict.

🇧🇪Belgium BramDriesen Belgium 🇧🇪

Thanks for the merge request!

I don't remember by hearth how notifications are stored, so I'll need to manually test this change.

🇧🇪Belgium BramDriesen Belgium 🇧🇪

Might actually be supported by this change: 🐛 Entity type id and bundle conflict Needs review

🇧🇪Belgium BramDriesen Belgium 🇧🇪

Also added a few change comments based of this duplicate issue 🐛 Entity type id and bundle conflict Needs review

🇧🇪Belgium BramDriesen Belgium 🇧🇪

From what I know, you don't need to have the bootstrap theme in order for this to work.

🇧🇪Belgium BramDriesen Belgium 🇧🇪

That's a breaking change I don't want to make :-)

🇧🇪Belgium BramDriesen Belgium 🇧🇪

bramdriesen made their first commit to this issue’s fork.

🇧🇪Belgium BramDriesen Belgium 🇧🇪

I think this is just 2 changes? I suggest closing this as duplicate and move the changes to #3326532

🇧🇪Belgium BramDriesen Belgium 🇧🇪

Committed! Will see if there is anything else ready and tag a new release.

🇧🇪Belgium BramDriesen Belgium 🇧🇪

Makes perfect sense!

Production build 0.71.5 2024