- 
                Notifications
    
You must be signed in to change notification settings  - Fork 4.6k
 
Remove useBlockDisplayTitle from BlockSwitcher #72608
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
base: trunk
Are you sure you want to change the base?
Conversation
useBlockDisplayTitle created two subscriptions and was running everytime content length changed. The main reason to use that hook is if you need to display the custom block title, which we don't. So, by switching to match?.title and blockType.title, we remove two subscriptions that were firing many times each keystroke in order to get a label for a button that isn't changing.
| 
          
 Warning: Type of PR label mismatch To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here. 
 Read more about Type labels in Gutenberg. Don't worry if you don't have the required permissions to add labels; the PR reviewer should be able to help with the task.  | 
    
| 
           The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the  If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.  | 
    
| 
           Size Change: +25 B (0%) Total Size: 2.19 MB 
 ℹ️ View Unchanged
  | 
    
| 
          
 Flaky tests detected in 20d0c96. 🔍  Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/18730717152 
  | 
    
| 
           It does bother me that we have two subscriptions for every block in   | 
    
| 
           The   | 
    
| // For BlockSwitcher, we only show the block type or variation name | ||
| // We don't need custom labels (metadata.name) - those are only for list view | ||
| _blockTitle = match?.title || blockType.title; | ||
| 
               | 
          ||
| // Truncate if needed (max 35 chars) | ||
| if ( _blockTitle && _blockTitle.length > 35 ) { | ||
| _blockTitle = _blockTitle.slice( 0, 34 ) + '…'; | ||
| } | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree this isn't the right change. I'd like it to be a fix within useBlockDisplayTitle. The architecture of needing two subscriptions that get retriggered on every keystroke to get a static title is bothersome to me 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good. I tried to optimize it in the past (#58250), so I'm happy to review changes when they're ready.

useBlockDisplayTitle created two subscriptions and was running everytime content length changed. The main reason to use that hook is if you need to display the custom block title, which we don't. So, by switching to match?.title and blockType.title, we remove two subscriptions that were firing many times each keystroke in order to get a label for a button that isn't changing.
What?
Closes
Why?
How?
Testing Instructions
Testing Instructions for Keyboard
Screenshots or screencast