Skip to content
This repository has been archived by the owner on Mar 13, 2024. It is now read-only.

[MM-45850]: Migrate dot menu to MUI menu and post reminders #12007

Merged
merged 28 commits into from
Mar 1, 2023

Conversation

AshishDhama
Copy link
Contributor

@AshishDhama AshishDhama commented Jan 17, 2023

Summary

Migrate the post dot menu to the MUI menu
Note: Opening up for early review, I May add a post reminder in this or another pr. Work is done.

Ticket Link

Fixes https://mattermost.atlassian.net/browse/MM-45850

Related Pull Requests

NA

Screenshots

Release Note

* Migrate post dot menu to MUI menu

@AshishDhama AshishDhama self-assigned this Jan 17, 2023
@AshishDhama AshishDhama changed the title [MM-45850]: post reminders [MM-45850]: Migrate dot menu to MUI menu and post reminders Jan 17, 2023
@AshishDhama AshishDhama added the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Jan 17, 2023
@AshishDhama
Copy link
Contributor Author

@M-ZubairAhmed, I think the only thing left is this new badge
Screenshot 2023-01-17 at 12 58 52 AM

leadingElement={<ClockOutlineIcon size={18}/>}
trailingElements={<ChevronRightIcon size={16}/>}
menuId={`remind_post_${props.post.id}-menu`}
forceOpenOnLeft={true}
Copy link
Member

Choose a reason for hiding this comment

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

is the force necessary here?

Copy link
Contributor Author

@AshishDhama AshishDhama Jan 17, 2023

Choose a reason for hiding this comment

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

Yeah, Not necessary earlier. When I was testing it, It wasn't working without it.
Maybe after your changes, it has started working without force left. Thanks for flagging.

Comment on lines 126 to 132
<h5 className={'postReminderMenuHeader'}>
{formatMessage(
{id: 'post_info.post_reminder.sub_menu.header',
defaultMessage: 'Set a reminder for:'},
)}
</h5>
Copy link
Member

Choose a reason for hiding this comment

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

should we make Menu.Header component? can be used in other menus as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems like one-off case to me


type Props = {
onExited: () => void;
userId: string;
Copy link
Member

@M-ZubairAhmed M-ZubairAhmed Jan 17, 2023

Choose a reason for hiding this comment

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

addPostReminder , userId and isMilitaryTime can be directly derived from PropsFromRedux

Comment on lines -766 to -779
JOIN_LEAVE: 'system_join_leave' as const,
JOIN_CHANNEL: 'system_join_channel' as const,
Copy link
Member

Choose a reason for hiding this comment

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

not sure why we did like this in first place

Copy link
Contributor

Choose a reason for hiding this comment

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

@M-ZubairAhmed I would guess that PostTypes.JOIN_LEAVE would be a const and could not be written over

@@ -2138,6 +2138,15 @@ export default class Client4 {
);
}

addPostReminder = (userId: string, postId: string, timestamp: number) => {
this.trackEvent('api', 'api_post_set_reminder');
Copy link
Member

Choose a reason for hiding this comment

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

is this new telemetry key made aware to Telemetry team?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sri-byte Please let me know, do I need to change anything here

Comment on lines 211 to 213
<button
className='PostReminderModal__input'
type='button'
Copy link
Member

@M-ZubairAhmed M-ZubairAhmed Jan 17, 2023

Choose a reason for hiding this comment

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

curious if we can use the new menu here as well or is that not prepared for it?

Copy link
Member

@M-ZubairAhmed M-ZubairAhmed left a comment

Choose a reason for hiding this comment

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

few comments

@AshishDhama
Copy link
Contributor Author

@matthewbirtch This is a known issue. @M-ZubairAhmed will add changes tomorrow
Screenshot 2023-01-17 at 5 42 01 PM

@AshishDhama AshishDhama force-pushed the MM-45850_post-reminders branch from 7de2a00 to af9628f Compare January 17, 2023 13:03
@AshishDhama AshishDhama added the 1: UX Review Requires review by a UX Designer label Jan 17, 2023
@esethna esethna requested review from asaadmahmood and removed request for matthewbirtch January 18, 2023 16:28
@esethna
Copy link
Contributor

esethna commented Jan 18, 2023

Adding @asaadmahmood for UX review given Matt's transition

@esethna esethna closed this Jan 18, 2023
@mattermost-build mattermost-build removed the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Jan 18, 2023
@esethna esethna reopened this Jan 18, 2023
@esethna esethna added the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Jan 18, 2023
@mm-cloud-bot mm-cloud-bot removed the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Jan 18, 2023
@mattermost-build
Copy link
Contributor

Successfully triggered E2E testing!
GitLab pipeline | Test dashboard

@furqanmlk furqanmlk self-requested a review February 28, 2023 20:16
@AshishDhama AshishDhama added 4: Reviews Complete All reviewers have approved the pull request and removed 3: QA Review Requires review by a QA tester labels Mar 1, 2023
@mm-cloud-bot
Copy link

New commit detected. SpinWick will upgrade if the updated docker image is available.

@mm-cloud-bot
Copy link

Mattermost test server updated with git commit 6a8b14243a80a9ceeff834ab48899be0a40a0ccc.

Access here: https://mattermost-webapp-pr-12007.test.mattermost.cloud

@AshishDhama AshishDhama merged commit c91f98c into master Mar 1, 2023
@AshishDhama AshishDhama deleted the MM-45850_post-reminders branch March 1, 2023 06:09
@mm-cloud-bot
Copy link

Test server destroyed

@mattermost-build mattermost-build removed the Setup Cloud Test Server Setup a test server using Mattermost Cloud label Mar 1, 2023
@mm-cloud-bot
Copy link

Test server destroyed

@amyblais amyblais added Changelog/Done Required changelog entry has been written Docs/Not Needed Does not require documentation Docs/Needed Requires documentation and removed Docs/Not Needed Does not require documentation labels Mar 1, 2023
@amyblais amyblais added this to the v7.10.0 milestone Mar 2, 2023
@justinegeffen justinegeffen added Docs/Done Required documentation has been written Docs/Needed Requires documentation and removed Docs/Needed Requires documentation Docs/Done Required documentation has been written labels Apr 11, 2023
justinegeffen added a commit to mattermost/docs that referenced this pull request Apr 11, 2023
@justinegeffen justinegeffen added Docs/Done Required documentation has been written and removed Docs/Needed Requires documentation labels Apr 11, 2023
justinegeffen added a commit to mattermost/docs that referenced this pull request Apr 12, 2023
* Create message-reminders.rst

Docs for: mattermost/mattermost-webapp#12007

* Update message-reminders.rst

* Update channels.rst

* Update message-reminders.rst

* Update message-reminders.rst
amyblais added a commit to mattermost/docs that referenced this pull request Apr 14, 2023
* Update conf.py

* Update deprecated-configuration-settings.rst (#6318)

Doc update for: mattermost/mattermost#22422

* Create message-reminders.rst (#6322)

* Create message-reminders.rst

Docs for: mattermost/mattermost-webapp#12007

* Update message-reminders.rst

* Update channels.rst

* Update message-reminders.rst

* Update message-reminders.rst

* Update boards content for 7.10 (#6320)

* boards update

* Update source/boards/getting-started.rst

Co-authored-by: Winson Wu <[email protected]>

* Update source/boards/getting-started.rst

Co-authored-by: Winson Wu <[email protected]>

* Update share-and-collaborate.rst

* Update share-and-collaborate.rst

* Update get-started-with-boards.rst

* Update source/boards/getting-started.rst

Co-authored-by: Winson Wu <[email protected]>

* Update source/boards/share-and-collaborate.rst

Co-authored-by: Winson Wu <[email protected]>

---------

Co-authored-by: Winson Wu <[email protected]>

* Added enable board sharing configs and others (#6326)

* Update cloud-subscriptions.rst

* Update editions-and-offerings.rst

* Update create-channels.rst

* Update search-for-messages.rst

* Update set-channel-preferences.rst

* Update share-files-in-messages.rst

* Create product-configuration-settings.rst

* Update administration.rst

* Update advanced-permissions.rst

* Update shared-channels.rst

* Update sso-gitlab.rst

* Update sso-gitlab.rst

* Update sso-office.rst

* Update plugins-configuration-settings.rst

* Update plugins-configuration-settings.rst

* Update product-configuration-settings.rst

* v7.10 Changelog (#6311)

* Update self-managed-changelog.md

* Update self-managed-changelog.md

* Update self-managed-changelog.md

* Update open-source-components.rst

* Update release-lifecycle.rst

* Update self-managed-changelog.md

* Update self-managed-changelog.md

* Update important-upgrade-notes.rst

* Update self-managed-changelog.md

* Update source/install/self-managed-changelog.md

Co-authored-by: Maria A Nunez <[email protected]>

* Update source/install/self-managed-changelog.md

Co-authored-by: Winson Wu <[email protected]>

* Apply suggestions from code review

Co-authored-by: Carrie Warner (Mattermost) <[email protected]>

* Update self-managed-changelog.md

* Update self-managed-changelog.md

* Update self-managed-changelog.md

* Update self-managed-changelog.md

* Update self-managed-changelog.md

* Update self-managed-changelog.md

* Update self-managed-changelog.md

---------

Co-authored-by: Maria A Nunez <[email protected]>
Co-authored-by: Winson Wu <[email protected]>
Co-authored-by: Carrie Warner (Mattermost) <[email protected]>

---------

Co-authored-by: Justine Geffen <[email protected]>
Co-authored-by: Winson Wu <[email protected]>
Co-authored-by: Maria A Nunez <[email protected]>
Co-authored-by: Carrie Warner (Mattermost) <[email protected]>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
4: Reviews Complete All reviewers have approved the pull request Changelog/Done Required changelog entry has been written Docs/Done Required documentation has been written release-note
Projects
None yet
Development

Successfully merging this pull request may close these issues.