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

For viewers, default to comment mode and hide non-read-only modes and actions #4609

Merged
merged 5 commits into from
Dec 7, 2023
Merged

For viewers, default to comment mode and hide non-read-only modes and actions #4609

merged 5 commits into from
Dec 7, 2023

Conversation

ruggi
Copy link
Contributor

@ruggi ruggi commented Dec 6, 2023 •
edited
Loading

Fix #4608

Problem:

Viewers cannot do or use most of the regular actions and modes, but the UI still shows them.
Also, the default mode when opening a project in view mode would make sense to be comment mode.

Fix:

  1. Switch to comment mode when loading the editor as a viewer
  2. Hide the toolbar buttons for insert mode and text mode
  3. Hide the secondary toolbar for edit mode
  4. Disable the insertion, text, swap, etc. hotkeys for viewers

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Dec 6, 2023 •
edited
Loading

Try me

@relativeci RelativeCI
Copy link

relativeci bot commented Dec 6, 2023 •
edited
Loading

Job #9485: Bundle Size — 66.22MiB (~+0.01%).

1c78ee9(current) vs 5957b21 master#9483(baseline)

Warning

Bundle contains 67 duplicate packages – View duplicate packages

Bundle metrics  Change 3 changes Regression 1 regression
                 Current
Job #9485		      Baseline
Job #9483
Regression  Initial JS 48.35MiB(~+0.01%) 48.35MiB
No change  Initial CSS 0B 0B
Change  Cache Invalidation 18.43% 18.67%
No change  Chunks 26 26
No change  Assets 30 30
Change  Modules 4490(+0.02%) 4489
No change  Duplicate Modules 480 480
No change  Duplicate Code 30.37% 30.37%
No change  Packages 466 466
No change  Duplicate Packages 67 67
Bundle size by type  Change 1 change Regression 1 regression
                 Current
Job #9485		      Baseline
Job #9483
Regression  JS 66.21MiB (~+0.01%) 66.2MiB
Not changed  HTML 11.32KiB 11.32KiB

View job #9485 report View feat/default-viewer-mode branch activity

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Dec 6, 2023 •
edited
Loading

Performance test results:
(Chart1)
(Chart2)

@ruggi ruggi marked this pull request as ready for review December 6, 2023 15:17
@balazsbajorics
Copy link
Contributor

image how do I reproduce? opening my own project in a private tab did not seem to work properly

@ruggi
Copy link
Contributor Author

ruggi commented Dec 6, 2023

how do I reproduce? opening my own project in a private tab did not seem to work properly

you're not signed in so it's not recognized as your project. I guess we could put it behind a FF but it should be done for other stuff too, like see the viewer badge in the bottom right 🤔

ruggi added 3 commits December 6, 2023 18:17
@ruggi
hide strategy indicator
313074b
@ruggi
open comment section
e254a3a
@ruggi
Merge branch 'master' into feat/default-viewer-mode
1c78ee9 
# Conflicts:
#	editor/src/components/canvas/controls/comment-indicator.tsx
#	editor/src/components/canvas/controls/comment-popup.tsx
@ruggi ruggi merged commit 212e6c0 into master Dec 7, 2023
9 checks passed
@ruggi ruggi deleted the feat/default-viewer-mode branch December 7, 2023 10:34
@ruggi ruggi mentioned this pull request Dec 7, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bkrmendy bkrmendy bkrmendy approved these changes

@seanparsons seanparsons seanparsons approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

for viewers, default to comment mode and hide non-read-only modes and actions
4 participants
@ruggi @balazsbajorics @seanparsons @bkrmendy