Skip to content

Refactoring - Extract buttonRendering from main initToolbar()#7819

Closed
CloCkWeRX wants to merge 1 commit into
wenzhixin:developfrom
CloCkWeRX:refactor
Closed

Refactoring - Extract buttonRendering from main initToolbar()#7819
CloCkWeRX wants to merge 1 commit into
wenzhixin:developfrom
CloCkWeRX:refactor

Conversation

@CloCkWeRX
Copy link
Copy Markdown

🤔Type of Request

  • Bug fix
  • New feature
  • Improvement
  • Documentation
  • Other

🔗Resolves an issue?

While trying to simply reverse the display order of when columns vs search was appended to the DOM in initToolbar, I found it difficult as everything was appending to html.

Minor refactor to split out a specific sub rendering behaviour to its own thing - more could be done, just doing one chunk at a time.

📝Changelog

  • Core
  • Extensions

💡Example(s)?

☑️Self Check before Merge

⚠️ Please check all items below before reviewing. ⚠️

  • Doc is updated/provided or not needed
  • Demo is updated/provided or not needed
  • Changelog is provided or not needed

@CloCkWeRX CloCkWeRX changed the title Extract buttonRendering from main initToolbar() Refactoring - Extract buttonRendering from main initToolbar() Apr 26, 2025
@CloCkWeRX CloCkWeRX marked this pull request as ready for review April 26, 2025 04:28
@CloCkWeRX CloCkWeRX mentioned this pull request Apr 26, 2025
11 tasks
@djhvscf
Copy link
Copy Markdown
Collaborator

djhvscf commented Apr 29, 2025

Please provide an Online Example to show this PR working, thanks!

@wenzhixin
Copy link
Copy Markdown
Owner

Hi @CloCkWeRX,

Thanks for this refactoring! I've created a similar PR (#8290) that includes the same extraction of renderButton from initToolbar, with one fix: the method call uses this.renderButton() instead of the bare renderButton() which would cause a ReferenceError.

That PR has been merged into develop, so I'll go ahead and close this one.

Thanks again for the initiative!

@wenzhixin wenzhixin closed this Apr 14, 2026
@CloCkWeRX CloCkWeRX deleted the refactor branch April 14, 2026 08:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants