Melbourne
Account created on 30 March 2015, almost 10 years ago
#

Merge Requests

Recent comments

🇦🇺Australia RichardGaunt Melbourne

The fix has been merged.
There is another outstanding issue with 11 we are working on and will have a fix shortly.

Thank you for reporting the issue and investigating

🇦🇺Australia RichardGaunt Melbourne

@smustgrave

No I would like it to be access driven rather than publish status similar to how MenuTree works

https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/navig...

🇦🇺Australia RichardGaunt Melbourne

I am happy to assist with the patch if it would be accepted but perhaps a general guidance on what method would be accepted etc.

🇦🇺Australia RichardGaunt Melbourne

@smustgrave reverted to array structure for dataprovider.

🇦🇺Australia RichardGaunt Melbourne

I've added test coverage for this change, also found a nit issue while testing the error messages.

The newline separator was incorrect meaning `/n` was showing in the separated error messages - figured it was closely enough related to this issue to be included.

🇦🇺Australia RichardGaunt Melbourne

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

🇦🇺Australia RichardGaunt Melbourne

@dhruv.mittal Could you create the PR from a forked https://github.com/civictheme/monorepo-drupal this way I can review and merge there. Changes from github get pushed up to this repository.

I can review and have behat tests added there.

🇦🇺Australia RichardGaunt Melbourne

Hi @dhruv.mittal,

Pull requests for this project are done on this repository via GitHub: https://github.com/civictheme/monorepo-drupal

There are three parts to this solution:

Update the civictheme_preprocess_fieldset__form_element__civictheme_field__checkboxes and civictheme_preprocess_fieldset__form_element__civictheme_field__radios
In `civictheme_preprocess_fieldset__form_element__civictheme_field__checkboxes`:

We need to change this line:
https://github.com/civictheme/monorepo-drupal/blob/develop/web/themes/co...
And
https://github.com/civictheme/monorepo-drupal/blob/develop/web/themes/co...

to check for `#value` rather than `#default_value`

Update the checkbox preprocessing

We need to change the checkbox preprocessing at: https://github.com/civictheme/monorepo-drupal/blob/develop/web/themes/co... to something like this

 if ($element['#type'] === 'checkbox') {
    $variables['control'][0]['is_checked'] = $element['#value'] ?? FALSE;
    $variables['control'][0]['is_checked'] = (bool) $variables['control'][0]['is_checked'];
  }

Add some behat tests
We need to add some tests ensuring that in error forms retain their values for checkboxes and radios

🇦🇺Australia RichardGaunt Melbourne

Yes the purpose is for this to be a utility to get values for a variety of different field types. As you say it only handles file type of media it would be good if it could be used for video

🇦🇺Australia RichardGaunt Melbourne

So from my understanding of your issue there is a need to implement an alter hook that allows the overriding / setting of the view filters.
Like:
`hook_civictheme_automated_list_view_arguments_alter` ?

🇦🇺Australia RichardGaunt Melbourne

We are making changes to how form's behave in 1.9 to fallback to Drupal if CivicTheme does not support a form element but this is not a form element we have plans for delivering first party support for at the moment.

🇦🇺Australia RichardGaunt Melbourne

@sime

This is our proposed fix.

Update civictheme_media_get_variables to handle the OEmbed (remote video).

We want to only return the properties that are useful for remote video:

<ul>
  <li>name</li>
  <li>url</li>
  <li>created</li>
  <li>updated</li>
  <li></li>
</ul>

Do not return other properties.

Does this align with your thinking?

🇦🇺Australia RichardGaunt Melbourne

A fix has been merged to develop and is being tested.

🇦🇺Australia RichardGaunt Melbourne

Hi Sonny,

This has been fixed in the 1.8 release and the form working being progressed at the moment is taking this into account.

🇦🇺Australia RichardGaunt Melbourne

@alex.skrypnyk title attribute only works when no text is in the tag and so yes your PR would work. The PR that has been committed
also works because it adds visually hidden text.

https://github.com/civictheme/uikit/pull/391

We might add your change to provide a tooltip for back to top as this would be nice feature to have but on this issue at hand both work correctly.

🇦🇺Australia RichardGaunt Melbourne

WE are actively working on this branch to create an opt-in system for form element preprocessing rather than the current opt-out.

https://github.com/civictheme/monorepo-drupal/pull/1306

The ck editor webform fields are something we are testing and ensuring are working before we are releasing a fix.

🇦🇺Australia RichardGaunt Melbourne

We have this issue being actively worked on, apologies for the delays.

🇦🇺Australia RichardGaunt Melbourne

@alan.cole Can you please review this and let me know if it solves your problem?

🇦🇺Australia RichardGaunt Melbourne

Thanks @amey and @sourojeetpaul.

Reviewing this change with @alancole: adding an `aria-label` as in your patch will duplicate the `aria-label` when the link opens in a new window and so we need to refine the solution slightly.

We are looking into the separate issue use of `aria-label` in the below issue as we believe we can remove the "Opens new window".

https://github.com/civictheme/uikit/issues/389

But for your issue of their not being an accessible back to top element we are suggesting the following:

Update back to top twig:

uikit/components/02-molecules/back-to-top/back-to-top.twig

<div class="ct-back-to-top" data-component-name="back-to-top" data-scrollspy data-scrollspy-offset="400">
  {% include '@atoms/button/button.twig' with {
    kind: 'link',
    type: 'primary',
    icon: 'up-arrow',
    url: '#top',
    text: '<span class="ct-visually-hidden">Return focus to the top of the page</span>',
    allow_html: true,
    modifier_class: 'ct-back-to-top__button',
  } only %}

This will provide a visually hidden text element as part of the button that will be read out. I will have @joshua1234511 create a PR in `uikit`

🇦🇺Australia RichardGaunt Melbourne

Josh is working on a fix:

1. Move the menu theming code into a helper _civictheme_preprocess_primary_navigation_menu and _civictheme_preprocess_footer_menu:
2. Add theme settings to define the primary navigations and the footer navigation menus
3. Update the menu name check to check for these theme settings if ($menu_name === 'THEME_SETTING')
4. Add update hook to add new theme setting or at least at a default value for expected values in this code above

Production build 0.71.5 2024