Zurich
Account created on 11 June 2012, almost 12 years ago
  • Senior Product Designer at GitLabΒ  …
#

Merge Requests

More

Recent comments

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

@teknocat great discovery, let's merge this! πŸ‘

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

I've pushed another change to include a proof of concept to move other actions to a kebab menu. It uses a container so it would be possible to move things in or out of that menu, but by default we would show all buttons which are not primary functions (submits) to that kebab menu.

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

@LRoels fixed, thank you!

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

See https://www.drupal.org/project/gin/issues/3293369 πŸ› Table header overflow issue for multiple user role on permissions page. Postponed: needs info

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

saschaeggi β†’ changed the visibility of the branch 3356717-moving-the-save to hidden.

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

I think this is a duplicate of https://www.drupal.org/project/gin/issues/3376559 πŸ› Home link added when using Toolbar Menu RTBC so I'm closing this in favor of the one we have in Gin.

Can you check if that MR fixes your issue and add a comment on that issue? Thank you!

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

Closing as duplicate of https://www.drupal.org/project/gin/issues/3395897 πŸ“Œ Use dependency injection in GinNavigation.php Needs review

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

@kksandr thanks for reporting the issue, I've pushed a fix.

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

Great, moving the status of this issue to outdated.

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

Consider this finally fixed πŸŽ‰

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

Fixed the overflow of the Powered by CKEditor badge and position.

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

Closing as I can't reproduce this issue.

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

Closing as this issue has no activity for a long time.

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

This was fixed in 3.x-dev with recent changes.

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

@kevinquillen is this still an issue with the latest Gin dev?

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

I went with a way simpler solution to fix this issue. Thanks y'all for participating here πŸ‘

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

Closing as I can't reproduce this and no steps have been provided to reproduce this.

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

Closing as haven't heard back. Feel free to reopen this issue if the issue reappears.

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

@robcarr how can I reproduce your scenario?

As using your overrides will certainly break it for the general implementation.

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

Is this reported problem already covered with https://www.drupal.org/project/toolbar_menu/issues/3271757 πŸ› css class name conflict with gin theme Needs review ?

Or maybe you can override the template for your custom menu?

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

This has been fixed in dev. Thanks for reporting πŸ‘

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

That's certainly a bug and shouldn't be the case. I'm moving this to Gin as the fix has to happen there.

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

Hey Traves, great improvement and thank you for spotting the PHP Warning. Let’s merge this πŸš‚

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

@finnsky I didn't saw that the Β«CreateΒ» menu is missing on mobile at first, thanks for pointing that out.

I've pushed a fix πŸ‘

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

Spacing looks as I would expect it on mobile, so I'm closing this one. But thanks for checking πŸ‘

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

Hey finnsky πŸ’ͺ

That's a great point. I've pushed a change to remove this override.

Thank you!

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

Hey finnsky πŸ‘‹

Is this OK in terms of user experience?

Yes, Gin enhances some features of the core navigation module.

We have to change some of the things to integrate it better with the amount of options the Gin Admin Theme has out of the box (different toolbar options, Darkmode, Accent color, high contrast mode etc.).

Here are some of the things just to keep it documented:

  • Our own icons
  • Integration with Darkmode, Accent color, high contrast mode etc.
  • Moved shortcuts to the top bar for now as we're providing this functionality there on all toolbar options

I'm closing this as works as designed.

Cheers!

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

Thanks, JΓΌrgen πŸ‘

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

«Good things take time» sorry for the long wait here, but we finally have it fixed and in a way so it should be future proof 🀝

Thanks everyone in participating here πŸ‘

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

saschaeggi β†’ changed the visibility of the branch 3355054-edge-case-illegal to active.

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

saschaeggi β†’ changed the visibility of the branch 3355054-edge-case-illegal to hidden.

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

Easy way to test this:

1. Add some messages via JS (can be done via the browser console):

const messages = new Drupal.Message();
const messageId = messages.add('test message');
messages.remove(messageId);
messages.add('test message', {type: 'status'});
messages.add('test message', {type: 'warning'});
messages.add('test message', {type: 'error'});

2. Test regular messages, e.g. Save form, clear cache etc.

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

This will be fixed with πŸ› Table header overflow issue for multiple user role on permissions page. Postponed: needs info

Closing this issue in favor of the other one and moving credits over.

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

New year, new approach πŸ˜…

This new approach should unify the markup of messages and also fix it in a future proof way so it will work with Drupal 11+ πŸŽ‰

Needs final testing πŸ‘―

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

saschaeggi β†’ changed the visibility of the branch 3214515-merged-with-existing-library to hidden.

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

I'm closing this as Gin is an admin theme and not a frontend theme (even though you can use it as one).

I'm pretty sure you'll need to inject the libraries. Best is to reach out in the Varbase issue queue. I know they're also using Gin heavily so they might know an easy solution for your issue.

Otherwise you might want to create a small helper module and inject the libraries yourself, see also https://www.drupal.org/docs/contributed-themes/gin-admin-theme/custom-th... β†’

Cheers

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

Closing again, as Core has a fix ready and we have different approaches in Gin's issue queue

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

How does this align or impact the changes from https://www.drupal.org/project/gin/issues/3293369 πŸ› Table header overflow issue for multiple user role on permissions page. Postponed: needs info ?

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

Closing this as I tested this on various sites without being able to reproduce this. Also a brand new Drupal instance just works as expected. You might need to debug your site ans check for errors.

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

This happens to me as well on 11.x-dev and the proposed solution seems to fix the problem

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

The behavior is as expected here as the tabs are part of that entity. We'll implement something similar in core for Claro.

Note that Gin is always ahead of Claro as we test features out first.

Regarding the message: From a UX POV it should appear right before (above) the action buttons in the content area.

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

See https://www.drupal.org/project/gin/issues/3293369 πŸ› Table header overflow issue for multiple user role on permissions page. Postponed: needs info for a solution

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

Thank you, let's close this then πŸ‘

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

The module works as intended. The goal of this module is to create a seamless login experience with either Claro or Gin.

For your usecase you can either create a small module to override/inject your library or you can easily utilize the built in gin-custom.css file which you could place your changes in.

You can find the documentation for this CSS file over at https://www.drupal.org/docs/contributed-themes/gin-admin-theme/custom-th... β†’

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

@phenaproxima no inconsistencies introduced here, rather eliminated as all our other navigation (toolbar) options use the same. You'll need our companion module (gin_toolbar) anyway to use Gin's option in your frontend theme.

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

As there is no major functionality broken caused by Gin I don't see this as a major issue (from Gin's side).

Also usually in production PHP errors are hidden so one more reason why it's not Β«majorΒ». Still something we have to look into to fix the Developer experience for sure.

Also I seem to be able to select the text:

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

Closing as duplicate of πŸ› Clientside validation breaks checkbox toggle Active

Can you check if the solution there will fix your issue as well?

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

@phenaproxima I've updated the description to reflect that, sorry forgot to mention that.

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

Not sure where you use these settings but they don't look like anything when used within Gin.

You have to use the ones within the Gin theme settings (/admin/appearance/settings/gin)

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

@jurgenhaas I've pushed a fix to πŸ“Œ Add support for Navigation Beta 1+ Active , please have another look πŸ‘€

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

@Esha_Kundu looking at this screenshot I'd rather like to see the module fix it's use of flex and wrap items. It's overlapping quite some items which is rather a bad UX. Also this way it will automatically fix the toggle. And that the toggle doesn't render correctly is a result of that.

So I will move this issue back to the module's issue queue to fix the apparent bug:

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

Closing as duplicate

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

This issue was caused by a recent change in core.

We have our own fix on the way with πŸ› Table header overflow issue for multiple user role on permissions page. Postponed: needs info

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

@robcarr I can't really reproduce this, can you provide steps how to reproduce this, Browser & which Drupal version you're using. Also does it happen only on a single instance (so potentially another module is interfering?)

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

I just tested this and the link just works fine.

We'll need more information here.

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

@JurriaanRoelofs definitely a mix between perfectionism and some things I really want to get in beforehand πŸ˜‰

But 2024 is the year we'll see the stable release for sure I promise πŸ˜…

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

@alexandru.dumitru would

if (!isset($variables['element']['#weight']) || !is_float($variables['element']['#weight'])) {

work to solve your issue?

PS: please use an MR instead of patches as we don't use patches anymore.

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

The problem being we need a way to identify checkbox groups, as Drupal doesn't provide an official way to do so.

See #2643012: Add property to identify if checkbox/radio is in group of checkboxes/radios β†’ , maybe we could check for the integer/float type instead if that would solve this issue, otherwise we might have to close this as won't fix πŸ‘€

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

FYI $suggestions[] = 'input__checkbox__toggle'; can't be removed as it will break core functionalities.

So maybe better to move this issue to the clientside_validation.

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

@Hebl I pushed a fix for Chrome πŸ‘€

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

Pushed another change to switch to use scrollsync to avoid some of the jerkyness when positioning an element with JS within overflow.

Testers wanted πŸ‘€

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

@Arantxio I was able to reproduce it in Chrome. I've pushed a fix.

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

I've setup some artificial roles in the linked Tugboat of the MR and can't reproduce the whitespace, see https://mr404-tvh4mpqiaqdfkkufdvfcc1xmmqdtjaxd.tugboatqa.com/admin/peopl... (admin/admin)

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

@Arantxio

it creates loads of blank space next to the permissions wrapper div

Can you share a screenshot?

kinda fixes this problem, however now everything gets cramped together which is also not wanted.

For this as well. Thank you.

πŸ‡¨πŸ‡­Switzerland saschaeggi Zurich

I've created a new branch with a complete new approach and for me it seems to work locally, but I'd love to gather some more feedback.

Can somebody please test the branch https://git.drupalcode.org/project/gin/-/merge_requests/404?

You can generate a patch to use against the latest dev with https://git.drupalcode.org/project/gin/-/merge_requests/404.diff

Thank you in advance!

Production build 0.69.0 2024