Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CIVIC-2089] Mobile search #1336

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from
Open

Conversation

febdao
Copy link
Collaborator

@febdao febdao commented Feb 14, 2025

Ticket:

Checklist before requesting a review

  • I have formatted the subject to include ticket number as Issue #123456 by drupal_org_username: Issue title
  • I have added a link to the issue tracker
  • I have provided information in Changed section about WHY something was done if this was not a normal implementation
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added tests that prove my fix is effective or that my feature works
  • I have run new and existing relevant tests locally with my changes, and they passed
  • I have provided screenshots, where applicable

Changed

  1. Added Show link in mobile menu field into search block type
  2. Implemented the search as mobile

Screenshots

Screenshot 2025-02-14 at 11 08 05 am
Screenshot 2025-02-14 at 11 07 01 am

@febdao febdao added the State: Needs review Pull requests needs a review from assigned developers label Feb 14, 2025
@febdao febdao requested a review from richardgaunt February 14, 2025 00:10
@febdao febdao self-assigned this Feb 14, 2025
@febdao febdao requested a review from alan-cole February 14, 2025 00:13
Copy link
Collaborator

@alan-cole alan-cole left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@richardgaunt - can you have a look at this and provide feedback on the best way to implement?

* @return array|null
* The stored link array, or NULL if no link is stored.
*/
function _civictheme_search_link(array $link = NULL): ?array {
Copy link
Collaborator

@alan-cole alan-cole Feb 14, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know about this approach. I'd leave it to @richardgaunt to provide feedback.

This seems like it could break if search block isn't rendered before the mobile navigation block, or if there are multiple search blocks.

I think we had discussed about multiple search blocks showing multiple links in mobile menu.

My assumption would have been something like:

use Drupal\block\Entity\Block;

// Load all blocks of type 'search'
$blocks = Block::loadMultiple(['type' => 'search']);

// Initialize an empty array to store the block data
$block_data = [];

// Iterate through the blocks and do something with them
foreach ($blocks as $block) {
  // Get the block's configuration
  $config = $block->getConfiguration();

  // Check if the "field_c_b_link_in_mobile_menu" field checkbox is checked
  $link_in_mobile_menu = $config['field_c_b_link_in_mobile_menu'] ?? false;

  // Get the block's URL and title
  $block_url = $config['url'];
  $block_title = $config['title'];

  if ($link_in_mobile_menu) {
    // Add the block's URL and title to the $block_data array
    $block_data[] = [
      'url' => $block_url,
      'title' => $block_title,
    ];
  }
}

// Now you can use the $block_data array to do something with the block data
print_r($block_data);

in the mobile menu navigation, to identify any search blocks with that option checked.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes this doesn't look right to me

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks @alan-cole for the suggestion.
I tried with $blocks = Block::loadMultiple(['type' => 'search']); but it didn't work for me so I use

  // Load all blocks of type 'civictheme_search'
  $block_storage = \Drupal::entityTypeManager()->getStorage('block_content');
  $blocks = $block_storage->loadByProperties(['type' => 'civictheme_search']);

to get the block search content, it seems work well.
Please review my change.
Thank you!

@febdao febdao requested a review from alan-cole March 7, 2025 04:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
State: Needs review Pull requests needs a review from assigned developers
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants