Skip to content

refactor: moved classic sidebar settings#1127

Merged
Soare-Robert-Daniel merged 2 commits into
developmentfrom
refactor/sidebar_settings
Aug 12, 2025
Merged

refactor: moved classic sidebar settings#1127
Soare-Robert-Daniel merged 2 commits into
developmentfrom
refactor/sidebar_settings

Conversation

@RaduCristianPopescu
Copy link
Copy Markdown
Contributor

Summary

Moved Feedzy classic sidebar settings to the top

Will affect visual aspect of the product

YES

Screenshots

CleanShot 2025-08-07 at 16 03 12@2x

Closes https://github.com/Codeinwp/feedzy-rss-feeds-pro/issues/863

@RaduCristianPopescu RaduCristianPopescu self-assigned this Aug 7, 2025
@RaduCristianPopescu RaduCristianPopescu added the pr-checklist-skip Allow this Pull Request to skip checklist. label Aug 7, 2025
@pirate-bot pirate-bot added the pr-checklist-complete The Pull Request checklist is complete. (automatic label) label Aug 7, 2025
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This refactor changes the conditional rendering approach for sidebar settings in the Feedzy block inspector, replacing React conditional rendering with CSS display toggling to move classic sidebar settings to the top.

  • Replaced conditional JSX rendering ({condition && <Component />}) with div wrappers using CSS display properties
  • Modified three main sections: content tab, style tab, and advanced tab rendering
  • Changed from React's conditional mounting/unmounting to CSS visibility control

Comment thread js/FeedzyBlock/inspector.js Outdated
</Button>
</PanelBody>
{'content' === this.state.tab && (
<div style={{ display: 'content' === this.state.tab ? 'block' : 'none' }}>
Copy link

Copilot AI Aug 7, 2025

Choose a reason for hiding this comment

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

Using CSS display toggle keeps all components mounted in the DOM even when hidden, which can impact performance compared to conditional rendering. The original approach ({condition && <Component />}) only renders components when needed, reducing memory usage and improving performance.

Suggested change
<div style={{ display: 'content' === this.state.tab ? 'block' : 'none' }}>
{this.state.tab === 'content' && (

Copilot uses AI. Check for mistakes.
Comment thread js/FeedzyBlock/inspector.js Outdated

{'fetched' === this.props.state.route &&
'style' === this.state.tab && (
<div style={{
Copy link

Copilot AI Aug 7, 2025

Choose a reason for hiding this comment

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

[nitpick] The inline style object with complex conditional logic spanning multiple lines reduces readability. Consider extracting this logic into a computed property or helper function for better maintainability.

Copilot uses AI. Check for mistakes.
Comment thread js/FeedzyBlock/inspector.js Outdated
Comment on lines +792 to +867
<div style={{
display: 'fetched' === this.props.state.route &&
'advanced' === this.state.tab ? 'block' : 'none'
Copy link

Copilot AI Aug 7, 2025

Choose a reason for hiding this comment

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

[nitpick] Similar to the previous comment, this complex conditional logic in inline styles makes the code harder to read and maintain. Consider consolidating the display logic into reusable helper functions.

Suggested change
<div style={{
display: 'fetched' === this.props.state.route &&
'advanced' === this.state.tab ? 'block' : 'none'
<div style={{
display: this.getDisplayStyle(
this.props.state.route,
this.state.tab,
'fetched',
'advanced'
)

Copilot uses AI. Check for mistakes.
@pirate-bot
Copy link
Copy Markdown
Contributor

pirate-bot commented Aug 7, 2025

Plugin build for 76b1392 is ready 🛎️!

Note

You can preview the changes in the Playground

@RaduCristianPopescu RaduCristianPopescu force-pushed the refactor/sidebar_settings branch from 868a76a to d2fe022 Compare August 8, 2025 07:34
@Soare-Robert-Daniel Soare-Robert-Daniel force-pushed the refactor/sidebar_settings branch from d2fe022 to fec15b0 Compare August 12, 2025 08:18
@Soare-Robert-Daniel Soare-Robert-Daniel merged commit bad9747 into development Aug 12, 2025
9 checks passed
@Soare-Robert-Daniel Soare-Robert-Daniel deleted the refactor/sidebar_settings branch August 12, 2025 08:37
@pirate-bot
Copy link
Copy Markdown
Contributor

🎉 This PR is included in version 5.1.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

@pirate-bot pirate-bot added the released Indicate that an issue has been resolved and released in a particular version of the product. label Aug 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-checklist-complete The Pull Request checklist is complete. (automatic label) pr-checklist-skip Allow this Pull Request to skip checklist. released Indicate that an issue has been resolved and released in a particular version of the product.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants