Skip to content

feat: added modal confirmation before pinning message - #31348

Merged
kodiakhq[bot] merged 15 commits into
RocketChat:developfrom
Sayan4444:pin_message_modal
Jan 5, 2024
Merged

feat: added modal confirmation before pinning message#31348
kodiakhq[bot] merged 15 commits into
RocketChat:developfrom
Sayan4444:pin_message_modal

Conversation

@Sayan4444

@Sayan4444 Sayan4444 commented Jan 1, 2024

Copy link
Copy Markdown
Contributor

Proposed changes (including videos or screenshots)

AFTER THE FIX
https://github.com/RocketChat/Rocket.Chat/assets/112304873/2e74a44f-e168-48af-817d-c7b945d1f5b0

REASON BEHIND CHANGES
Made the same changes as done by DISCORD to tackle this issue.

Issue(s)
This should close issues :-
closes #30777

Steps to test or reproduce

  1. Go to Teams/Channel
  2. Click on any of the teams/Channels where you are participant.
  3. Try to Send a Text to the Team/Channel
  4. Try to Edit the Text and See that there is no Cancel Button for Edit Text if the user wants to continue without editing the Text.]

@Sayan4444
Sayan4444 requested a review from a team as a code owner January 1, 2024 06:04
@changeset-bot

changeset-bot Bot commented Jan 1, 2024

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: 327b50f

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 30 packages
Name Type
@rocket.chat/meteor Minor
@rocket.chat/core-typings Minor
@rocket.chat/rest-typings Minor
@rocket.chat/core-services Patch
@rocket.chat/cron Patch
@rocket.chat/gazzodown Major
@rocket.chat/livechat Patch
@rocket.chat/model-typings Patch
@rocket.chat/ui-contexts Major
@rocket.chat/account-service Patch
@rocket.chat/authorization-service Patch
@rocket.chat/ddp-streamer Patch
@rocket.chat/omnichannel-transcript Patch
@rocket.chat/presence-service Patch
@rocket.chat/queue-worker Patch
@rocket.chat/stream-hub-service Patch
@rocket.chat/api-client Patch
@rocket.chat/license Patch
@rocket.chat/omnichannel-services Patch
@rocket.chat/pdf-worker Patch
@rocket.chat/presence Patch
rocketchat-services Patch
@rocket.chat/ddp-client Patch
@rocket.chat/fuselage-ui-kit Major
@rocket.chat/models Patch
@rocket.chat/ui-client Major
@rocket.chat/ui-video-conf Major
@rocket.chat/uikit-playground Patch
@rocket.chat/web-ui-registration Major
@rocket.chat/instance-status Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@Sayan4444 Sayan4444 changed the title feature: Added modal confirmation before pinning message refactor: Added modal confirmation before pinning message Jan 1, 2024
@codecov

codecov Bot commented Jan 1, 2024

Copy link
Copy Markdown

Codecov Report

Attention: 15 lines in your changes are missing coverage. Please review.

Comparison is base (9c59a87) 59.38% compared to head (327b50f) 49.36%.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##           develop   #31348       +/-   ##
============================================
- Coverage    59.38%   49.36%   -10.03%     
============================================
  Files         1793     3311     +1518     
  Lines        34909    81366    +46457     
  Branches      7246    16661     +9415     
============================================
+ Hits         20731    40163    +19432     
- Misses       12577    36501    +23924     
- Partials      1601     4702     +3101     
Flag Coverage Δ
e2e 52.53% <6.25%> (-0.04%) ⬇️
e2e-api 40.68% <ø> (?)
unit 76.67% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

@Sayan4444 Sayan4444 changed the title refactor: Added modal confirmation before pinning message Fix: Added modal confirmation before pinning message Jan 1, 2024
@Sayan4444 Sayan4444 changed the title Fix: Added modal confirmation before pinning message fix: Added modal confirmation before pinning message Jan 1, 2024
@Sayan4444 Sayan4444 changed the title fix: Added modal confirmation before pinning message feat: Added modal confirmation before pinning message Jan 1, 2024
@Sayan4444 Sayan4444 changed the title feat: Added modal confirmation before pinning message feat: added modal confirmation before pinning message Jan 1, 2024
Comment thread apps/meteor/client/startup/actionButtons/pinMessage.ts Outdated
Comment thread apps/meteor/client/startup/actionButtons/pinMessage.ts

@dougfabris dougfabris left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@Sayan4444 This is the desired result shared by our design team, please check if you can achieve it using <GenericModal />, if you have any questions, let me know!

Also, can you check what happened with the en.i18n.json?

@dougfabris dougfabris added this to the 6.6 milestone Jan 3, 2024
@Sayan4444

Sayan4444 commented Jan 3, 2024

Copy link
Copy Markdown
Contributor Author

@dougfabris Is this good enough or do you want me to add or correct anything else??

Screenshot 2024-01-03 224853

Can you tell me what is wrong about en.i18n.json?, it will be of greate help

@Sayan4444
Sayan4444 requested a review from dougfabris January 4, 2024 17:33
dougfabris
dougfabris previously approved these changes Jan 4, 2024

@dougfabris dougfabris left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@Sayan4444 Thank you so much for the contribution, it was very precise and valuable.
About the en.i18n.json file I think it was some issue syncing your code or with lint.
I made some small changes, just to match our standards and code organization.
Thanks again and I hope to see you contributing more 🚀

dougfabris
dougfabris previously approved these changes Jan 4, 2024
@Sayan4444

Copy link
Copy Markdown
Contributor Author

@dougfabris Yep I will be contributing more as much as I can as I love solving issues which directly impact the end user experience. Do you have an issue which u are also waiting long time to see someone working on it?

@dougfabris

Copy link
Copy Markdown
Member

@Sayan4444 That's awesome to hear from you.
Feel free to navigate through our repository and have fun with whatever you want, we found your choices amazing and we'll be reviewing them as soon as possible!

@dionisio-bot dionisio-bot Bot added stat: ready to merge PR tested and approved waiting for merge and removed stat: needs QA labels Jan 5, 2024
@dionisio-bot dionisio-bot Bot added stat: ready to merge PR tested and approved waiting for merge and removed stat: ready to merge PR tested and approved waiting for merge labels Jan 5, 2024
@kodiakhq
kodiakhq Bot merged commit 47331ba into RocketChat:develop Jan 5, 2024
@Sayan4444
Sayan4444 deleted the pin_message_modal branch January 5, 2024 15:51
gabriellsh added a commit that referenced this pull request Jan 5, 2024
…hideUi

* 'develop' of github.com:RocketChat/Rocket.Chat:
  chore: add gitpod config file for quick setup (#30921)
  feat: Allow user to set timeout of outgoing webhook (#31222)
  feat: added modal confirmation before pinning message (#31348)
  fix: not being able to access or create rooms with join code (#31270)
  chore: services changes on lifecycle methods + throw if svc is not available (#31375)
  test: bump playwright (#31376)
  i18n: update translations (#29462)
  i18n: adds video call translations for Persian language (#30406)
  regression: `AppRow` bundleIn verification (#31373)
  feat: Add Desktop PDF viewer (#31279)
  fix: Attachments not collapsing when using incoming webhook (#31318)
  chore: add aria-label to Select Inputs at Engagement Dashboard (#31249)
  fix: engagement dashboard timezone selector (#31248)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

community stat: ready to merge PR tested and approved waiting for merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Usability: The pin button is too easy to accidentally press and clogs up the chat

4 participants