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

Pagination hidden content fix #5590

Merged
merged 11 commits into from
May 23, 2024
Merged

Pagination hidden content fix #5590

merged 11 commits into from
May 23, 2024

Conversation

elzadj
Copy link
Collaborator

@elzadj elzadj commented May 21, 2024

Description

Fixes several issues with CL and pagination.

  1. Fixes broken icon making content hidden for footers 103, 104, 105 + theme footer.
  2. Allows to use empty groups / columns (without DC blocks inside) for bg.
  3. Fixes pagination bugs with blog patterns 12 and 24 (need to enable DC for all blocks).

How Has This Been Tested?

Test pagination and block patterns in general.
Test patterns with the listed footers.
Pure Footer Light PFL-PRO-105-broken-location-icon.txt

Test patterns with empty content for special bg.

Test checklist

_ Front/Back Testing _

  • Test the component in normal state in sidebar
  • Test the component in hover state in sidebar (if hover exists)
  • Check that the settings work on frontend
  • Check that backend works and saves the settings after the editor reload
  • Same 1-4 points for the toolbar
  • Same 1-4 points on responsive
  • Test the block inside the grid (Container + Row + Column)
  • Test the block as a standalone block
  • Duplicate the block, test that the settings of the first do not affect the second
  • Test with 2 blocks of the same type

_ Pre-Code Testing _

  • Test the component in normal state in sidebar
  • Test the component in hover state in sidebar (if hover exists)
  • Check that the settings work on frontend
  • Check that backend works and saves the settings after the editor reload
  • Same 1-4 points for the toolbar
  • Same 1-4 points on responsive
  • Test the block inside the grid (Container + Row + Column)
  • Test the block as a standalone block
  • Duplicate the block, test that the settings of the first do not affect the second
  • Test with 2 blocks of the same type
  • Check no commented code and no unnecessary imports
  • Standards of the project have been followed
  • No errors/warnings on console

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • My changes generate no new warnings/errors
  • I have added/updated tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

Summary by CodeRabbit

  • New Features

    • Enhanced dynamic block rendering to support additional parameters and improved content visibility management.
  • Bug Fixes

    • Improved handling of inner blocks to ensure accurate rendering and visibility based on attributes.
  • Performance Improvements

    • Optimized recursive checks for inner blocks, leading to more efficient content rendering.

@elzadj elzadj requested a review from Olekrut May 21, 2024 13:02
@elzadj elzadj self-assigned this May 21, 2024
@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label May 21, 2024
Copy link
Contributor

coderabbitai bot commented May 21, 2024

Walkthrough

The recent updates enhance the MaxiBlock and MaxiDynamicContent classes by introducing new methods for handling inner blocks and improving dynamic content rendering. Key changes include adding a $block parameter to relevant methods, updating logic to check inner blocks recursively, and managing content visibility based on specific attributes and conditions.

Changes

File Path Change Summary
core/blocks/class-maxi-block.php render_block method now accepts an additional $block parameter and updates render_dc call.
core/class-maxi-dynamic-content.php Added check_inner_blocks and recursive_check methods, updated render_dc and render_dc_classes methods, and introduced check_if_content_is_empty method.

🐇
In the code where blocks reside,
New methods now do guide.
With inner checks and dynamic flair,
Content renders with greater care.
A rabbit's touch, a coder's dream,
Together, we make the perfect team!
🐰


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@dosubot dosubot bot added the bug 🐛 Something isn't working label May 21, 2024
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits Files that changed from the base of the PR and between 12c8aba and 75c064d.
Files selected for processing (2)
  • core/blocks/class-maxi-block.php (2 hunks)
  • core/class-maxi-dynamic-content.php (3 hunks)
Additional comments not posted (2)
core/blocks/class-maxi-block.php (1)

265-273: The updated render_block method now correctly handles dynamic content based on the block type. Good use of the new $block parameter.

Verification successful

The render_block method is correctly set up to receive the $block parameter based on the usage of register_block_type in WordPress, which ensures the callback receives the necessary parameters.

  • core/blocks/class-maxi-block.php: The render_callback is set correctly to use render_block.
Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify all calls to `render_block` include the new `$block` parameter.

# Test: Search for the function usage. Expect: Only occurrences of the new signature.
rg --type php $'render_block'

Length of output: 340



Script:

#!/bin/bash
# Description: Search for calls to `render_block` and inspect the context to verify the usage of the new `$block` parameter.

# Test: Search for function calls. Expect: Calls with the new signature including the `$block` parameter.
rg --type php 'render_block\('

Length of output: 133



Script:

#!/bin/bash
# Description: Verify the context in which `render_block` is used as a callback and ensure the `$block` parameter is passed correctly.

# Test: Search for the usage of 'render_callback' to understand how `render_block` is invoked.
rg --type php 'render_callback'

Length of output: 126



Script:

#!/bin/bash
# Description: Search for block registration or related functions to understand how `render_callback` is used and verify if the `$block` parameter is passed during the callback invocation.

# Test: Search for block registration functions.
rg --type php 'register_block_type'

Length of output: 207



Script:

#!/bin/bash
# Description: Extract the relevant code around the first occurrence of `register_block_type` in `core/blocks/class-maxi-block.php` to understand the block registration and callback setup.

# Test: Extract 10 lines before and after the occurrence to get the context.
rg --type php -A 10 -B 10 'register_block_type' core/blocks/class-maxi-block.php

Length of output: 967



Script:

#!/bin/bash
# Description: Verify the implementation or documentation of `register_block_type` to ensure it passes the `$block` parameter to the callback.

# Test: Search for the definition or documentation of `register_block_type`.
rg --type php 'function register_block_type' 

Length of output: 46

core/class-maxi-dynamic-content.php (1)

542-559: Ensure proper handling of dc-status attribute in render_dc.

The logic in render_dc seems to rely heavily on the dc-status attribute. It would be beneficial to verify that this attribute is consistently handled across all relevant parts of the codebase to ensure uniform behavior.

Verification successful

The dc-status attribute is handled in multiple files within the codebase. Here are the key points:

  • In core/class-maxi-dynamic-content.php, the attribute is checked and used in various conditions.
  • In core/class-maxi-styles.php, the attribute is extracted from properties with a default value of false.
  • In core/blocks/class-maxi-block.php, the attribute is defined as a boolean type within the $dynamic_content_attributes array.

These usages indicate that the dc-status attribute is consistently handled across the codebase, ensuring proper checks and default values where necessary.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify that the `dc-status` attribute is properly handled across all usages in the codebase.

# Test: Search for the usage of `dc-status` attribute. Expect: Proper checks and handling in all occurrences.
rg --type php $'dc-status'

Length of output: 703



Script:

#!/bin/bash
# Extract and review the relevant code snippets for `dc-status` handling in core/class-maxi-styles.php and core/blocks/class-maxi-block.php

# Extract the relevant code snippet from core/class-maxi-styles.php
echo "Code snippet from core/class-maxi-styles.php:"
rg --context 5 --type php $'dc-status' core/class-maxi-styles.php

# Extract the relevant code snippet from core/blocks/class-maxi-block.php
echo "Code snippet from core/blocks/class-maxi-block.php:"
rg --context 5 --type php $'dc-status' core/blocks/class-maxi-block.php

Length of output: 1110

core/class-maxi-dynamic-content.php Outdated Show resolved Hide resolved
core/class-maxi-dynamic-content.php Show resolved Hide resolved
@elzadj elzadj changed the base branch from master to 1.8.5 May 21, 2024 14:20
Copy link
Collaborator

@Olekrut Olekrut left a comment

Choose a reason for hiding this comment

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

point 3
№12
image
author
№24
image

@elzadj
Copy link
Collaborator Author

elzadj commented May 22, 2024

point 3

@Olekrut
I've noticed that both "Posted by" and the divider don't have DC enabled from the cloud, could it be the reason? If not, please provide code for both, or a screenshot of the CL settings for both.

screenshot-wordpress local-2024 05 22-11_22_55
screenshot-wordpress local-2024 05 22-11_20_11

@elzadj elzadj requested a review from Olekrut May 22, 2024 08:39
Copy link
Collaborator

@Olekrut Olekrut left a comment

Choose a reason for hiding this comment

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

@Olekrut Olekrut self-requested a review May 22, 2024 10:02
@dosubot dosubot bot added the lgtm This PR has been approved by maintainer label May 22, 2024
@Olekrut Olekrut self-requested a review May 22, 2024 10:15
Copy link
Collaborator

@Olekrut Olekrut left a comment

Choose a reason for hiding this comment

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

image
empty column is not hidden
previous bugs are fixed

@dosubot dosubot bot removed the lgtm This PR has been approved by maintainer label May 22, 2024
@elzadj elzadj changed the base branch from 1.8.5 to master May 22, 2024 11:35
@elzadj elzadj requested a review from Olekrut May 22, 2024 11:37
Copy link
Collaborator

@Olekrut Olekrut left a comment

Choose a reason for hiding this comment

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

Hidden for blogs are not working correct after changing cat in CL
https://jam.dev/c/bf0af755-3364-4f8e-be86-af8ce0c197df

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits Files that changed from the base of the PR and between 75c064d and bf74efa.
Files selected for processing (1)
  • core/class-maxi-dynamic-content.php (5 hunks)
Additional comments not posted (2)
core/class-maxi-dynamic-content.php (2)

2292-2332: Review and possibly refactor check_if_content_is_empty for performance and clarity.

This comment was previously made and is still valid. The refactor simplifies the initial checks by consolidating them into a single condition and improves the regex patterns for replacing invalid parts of the data-stroke attribute. It also enhances readability by reducing nested conditions and clarifying the flow of logic.


553-571: Ensure proper handling of dc-status in render_dc.

This script searches for the usage of dc-status across PHP files to ensure it is consistently handled.

Verification successful

The detailed context around the usage of dc-status shows that it is consistently handled across different parts of the codebase. Here are the key observations:

  • Definition: dc-status is defined as a boolean attribute in core/blocks/class-maxi-block.php.
  • Usage in core/class-maxi-dynamic-content.php:
    • It checks if dc-status exists in the attributes.
    • It handles the case where dc-status is set to true within inner blocks.
    • It processes content based on the value of dc-status.
  • Usage in core/class-maxi-styles.php:
    • It extracts dc-status from properties with a default value of false.

These observations confirm that dc-status is properly handled in the render_dc method and across the codebase.

Scripts Executed

The following scripts were executed for the analysis:


Script:

#!/bin/bash
# Description: Verify that `dc-status` is properly handled across different blocks.

# Test: Search for the usage of `dc-status` in PHP files. Expect: Consistent handling across files.
rg --type php $'dc-status'

Length of output: 703



Script:

#!/bin/bash
# Description: Gather detailed context around each usage of `dc-status` in the codebase.

# Extract lines around each match to understand the context.
rg --type php --context 5 'dc-status'

Length of output: 4755

core/class-maxi-dynamic-content.php Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits Files that changed from the base of the PR and between bf74efa and 5992aee.
Files selected for processing (1)
  • core/class-maxi-dynamic-content.php (5 hunks)
Files skipped from review as they are similar to previous changes (1)
  • core/class-maxi-dynamic-content.php

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review Details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits Files that changed from the base of the PR and between 5992aee and e704b27.
Files selected for processing (1)
  • core/class-maxi-dynamic-content.php (5 hunks)
Files skipped from review as they are similar to previous changes (1)
  • core/class-maxi-dynamic-content.php

@elzadj elzadj requested a review from Olekrut May 23, 2024 08:56
@dosubot dosubot bot added the lgtm This PR has been approved by maintainer label May 23, 2024
@elzadj elzadj merged commit 3d29fa9 into master May 23, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something isn't working Final code review 🙏 high priority lgtm This PR has been approved by maintainer size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants