Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[BREAK] Remove message view mode from User Preferences #27867

Merged
merged 14 commits into from
Feb 9, 2023
Merged

[BREAK] Remove message view mode from User Preferences #27867

merged 14 commits into from
Feb 9, 2023

Conversation

hugocostadev
Copy link
Contributor

Proposed changes (including videos or screenshots)

Since the view mode setting in User preferences -> Messages -> View Mode does not work with the new messages template it will be removed

It was removed from every file and it was created a migration to remove from user obj in DB

Issue(s)

Steps to test or reproduce

Further comments

TC-367

@hugocostadev hugocostadev requested review from a team as code owners January 27, 2023 12:25
sampaiodiego
sampaiodiego reviewed Jan 27, 2023
View reviewed changes
Copy link
Member

@sampaiodiego sampaiodiego left a comment

Choose a reason for hiding this comment

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

This is a [BREAK] change right?

@hugocostadev
Copy link
Contributor Author

This is a [BREAK] change right?

I'm not sure since this feature is not working in the new message-parser and the old parser (Legacy Message Template) is going to be removed... That PR will remove should be [BREAK], right? WDYT?

@sampaiodiego
Copy link
Member

This is a [BREAK] change right?

I'm not sure since this feature is not working in the new message-parser and the old parser (Legacy Message Template) is going to be removed... That PR will remove should be [BREAK], right? WDYT?

it is not working due to a bug (from the user perspective).. but actually the feature (to control the message rendering size) is being removed (it was already silently removed actually, since the new message components do not support it), which means it is a breaking change.

@hugocostadev
Copy link
Contributor Author

This is a [BREAK] change right?

I'm not sure since this feature is not working in the new message-parser and the old parser (Legacy Message Template) is going to be removed... That PR will remove should be [BREAK], right? WDYT?

it is not working due to a bug (from the user perspective).. but actually the feature (to control the message rendering size) is being removed (it was already silently removed actually, since the new message components do not support it), which means it is a breaking change.

Got it! Thanks a lot for you explanation, I'll edit it here! ;D

@hugocostadev hugocostadev changed the title Chore: remove message view mode from User Preferences [BREAK] Remove message view mode from User Preferences Jan 27, 2023
@hugocostadev hugocostadev added this to the 6.0.0 milestone Jan 27, 2023
@codecov
Copy link

codecov bot commented Jan 27, 2023 •
edited
Loading

Codecov Report

Merging #27867 (33986c4) into develop (85e2af9) will increase coverage by 0.95%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop   #27867      +/-   ##
===========================================
+ Coverage    41.70%   42.65%   +0.95%     
===========================================
  Files          849      822      -27     
  Lines        17556    17039     -517     
  Branches      2087     2012      -75     
===========================================
- Hits          7321     7268      -53     
+ Misses        9962     9499     -463     
+ Partials       273      272       -1
Flag Coverage Δ
e2e 42.65% <ø> (+0.95%) ⬆️

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

yash-rajpal
yash-rajpal previously approved these changes Jan 31, 2023
View reviewed changes
@dionisio-bot dionisio-bot bot added stat: ready to merge PR tested and approved waiting for merge and removed stat: conflict labels Feb 6, 2023
yash-rajpal
yash-rajpal previously approved these changes Feb 7, 2023
View reviewed changes
@kodiakhq kodiakhq bot removed the stat: ready to merge PR tested and approved waiting for merge label Feb 8, 2023
@kodiakhq
Copy link
Contributor

kodiakhq bot commented Feb 8, 2023

This PR currently has a merge conflict. Please resolve this and then re-add the ['stat: ready to merge', 'automerge'] label.

@dionisio-bot dionisio-bot bot added stat: ready to merge PR tested and approved waiting for merge and removed stat: conflict labels Feb 8, 2023
yash-rajpal
yash-rajpal previously approved these changes Feb 8, 2023
View reviewed changes
rodrigok
rodrigok previously approved these changes Feb 8, 2023
View reviewed changes
@rodrigok rodrigok removed the request for review from sampaiodiego February 8, 2023 19:53
@kodiakhq kodiakhq bot removed the stat: ready to merge PR tested and approved waiting for merge label Feb 8, 2023
@kodiakhq
Copy link
Contributor

kodiakhq bot commented Feb 8, 2023

This PR currently has a merge conflict. Please resolve this and then re-add the ['stat: ready to merge', 'automerge'] label.

@rodrigok rodrigok dismissed stale reviews from yash-rajpal and themself via 53fc915 February 8, 2023 21:06
@dionisio-bot dionisio-bot bot added stat: ready to merge PR tested and approved waiting for merge and removed stat: conflict labels Feb 8, 2023
@rodrigok
Merge remote-tracking branch 'origin/develop' into chore/remove_messa…
b8943d1 
…ge_viewMode

# Conflicts:
#	apps/meteor/server/startup/migrations/v284.ts
@rodrigok rodrigok force-pushed the chore/remove_message_viewMode branch from 53fc915 to b8943d1 Compare February 8, 2023 21:09
@rodrigok
Copy link
Member

rodrigok commented Feb 9, 2023

@yash-rajpal can you review this PR again?

@kodiakhq kodiakhq bot merged commit 082c048 into develop Feb 9, 2023
@kodiakhq kodiakhq bot deleted the chore/remove_message_viewMode branch February 9, 2023 17:01
gabriellsh added a commit that referenced this pull request Feb 13, 2023
@gabriellsh
Merge branch 'develop' of github.com:RocketChat/Rocket.Chat into chor…
af95926 
…e/wizard

* 'develop' of github.com:RocketChat/Rocket.Chat:
  [BREAK] Unify monolith and microservices intercommunication on same technology and limit to Enterprise only (#27969)
  [NEW] Emojis rendering properly on quote message previews (#27972)
  Chore: Wait message to not be Busy (#28000)
  Chore: e2e changes tests (#27987)
  [IMPROVE] Audit (#27994)
  [IMPROVE] Registration Experience (#27820)
  [BREAK][ENTERPRISE] Limit presence statuses to 200 concurrent users when running monolith to keep performance (#27854)
  [BREAK] Removed deprecated settings for Legacy Messages, Marked, Snippet Messages, Autolinker and IssueLinks (#27902)
  [NEW] Permission to bypass message editing and removing limits (#27644)
  [FIX] Video Conf Message Blocks not always updating when running on micro services (#27764)
  Chore: Set missing colors (#27817)
  Fix CI
  Chore: undo busy selector
  [BREAK] Remove message view mode from User Preferences (#27867)
  Chore: Aria busy indicators (#27978)
  Chore: Refactor Composer Dropdown (#27931)
  Chore: Added Enterprise Options to Login customization (#27855)
  [FIX] Ignored messages reactivity (#27279)
  [BREAK] [NEW] Custom roles upsell modal (#27707)
@sampaiodiego sampaiodiego mentioned this pull request Feb 17, 2023
Merged
@sampaiodiego sampaiodiego mentioned this pull request Mar 9, 2023
Merged
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gabriellsh gabriellsh gabriellsh left review comments

@sampaiodiego sampaiodiego sampaiodiego left review comments

@yash-rajpal yash-rajpal yash-rajpal approved these changes

@rodrigok rodrigok rodrigok approved these changes

Assignees
No one assigned
Labels
squad: team-collab stat: QA tested stat: ready to merge PR tested and approved waiting for merge
Projects
None yet
Milestone
6.0.0
Development

Successfully merging this pull request may close these issues.
6 participants
@hugocostadev @sampaiodiego @rodrigok @gabriellsh @yash-rajpal @jessicaschelly