Skip to content

Conversation

@abbas-dependable-naqvi
Copy link
Contributor

Summary

Remove depricated registerPostDropdownMenuComponent

@abbas-dependable-naqvi
Copy link
Contributor Author

@wiggin77
Can you help us with this issue?
image (12)

@wiggin77
Copy link
Member

wiggin77 commented Aug 5, 2025

@wiggin77 Can you help us with this issue? image (12)

@hmhealey beyond replacing the deprecated registerPostDropdownMenuComponent API call with the new one, is there something else that needs to be done? It appears to not be a 1:1 replacement.

@abbas-dependable-naqvi abbas-dependable-naqvi added 2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester and removed Work In Progress Not yet ready for review labels Aug 14, 2025
Comment on lines 9 to 21
return (
<li
className='MenuItem'
role='menuitem'
>
<button className='style--none'>
<span className='MenuItem__icon'>
<GitHubIcon type='menu'/>
</span>
{'Create GitHub Issue'}
</button>
);

return (
<li
className='MenuItem'
role='menuitem'
>
{content}
</li>
);
}
</li>
);
Copy link
Member

Choose a reason for hiding this comment

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

The li and button elements are already rendered by the web app, so you can remove them from here. The span with MenuItem__icon also isn't actually needed because GitHubIcon already includes that inside it, so this should be:

    return (
        <>
            <GitHubIcon type='menu'/>
            {'Create GitHub Issue'}
        </>
    );

Testing that locally, the icon is a bit misaligned with the text, but that's fine since it's something I can fix on the web app side (https://mattermost.atlassian.net/browse/MM-65101)

Screenshot 2025-08-20 at 6 10 03 PM

<GitHubIcon type='menu'/>
export default function AttachCommentToIssuePostMenuAction() {
return (
<li
Copy link
Member

Choose a reason for hiding this comment

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

The same change should be made here as well

wiggin77

This comment was marked as outdated.

@wiggin77 wiggin77 requested review from hmhealey and wiggin77 August 21, 2025 22:12
Copy link
Member

@hmhealey hmhealey left a comment

Choose a reason for hiding this comment

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

Looks to me like that should work!

@abbas-dependable-naqvi
Copy link
Contributor Author

Looks to me like that should work!

Yup, working fine for me. Lets put it up for QA testing.
@arush-vashishtha Please test this

Copy link

@arush-vashishtha arush-vashishtha left a comment

Choose a reason for hiding this comment

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

This PR is tested & all the code changes are working fine.
LGTM

@abbas-dependable-naqvi abbas-dependable-naqvi merged commit 8ef1e43 into master Sep 1, 2025
11 checks passed
@abbas-dependable-naqvi abbas-dependable-naqvi deleted the MM-63447 branch September 1, 2025 07:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

2: Dev Review Requires review by a core committer 3: QA Review Requires review by a QA tester

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants