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.

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

Add the ability to lock NotebookTab layout #3627

Merged
merged 10 commits into from Mar 26, 2022

Conversation

Infinitay
Copy link
Contributor

@Infinitay Infinitay commented Mar 24, 2022

Pull request checklist:

  • CHANGELOG.md was updated, if applicable

Description

Gives users the ability to lock their tabs on Chatterino to prevent accidental movement.

chrome_kk9JcKz9g9.mp4

Additional Comments

I'm not sure if it would be better practice to rely only on the ChatterinoSetting rather than also using a private variable lockNotebookLayout_. Would appreciate some guidance.

@Infinitay
Copy link
Contributor Author

Infinitay commented Mar 24, 2022

I considered adding the menu action to NotebookTabs themselves, but midway I realized maybe it wouldn't be an ideal implementation. Originally, I considered giving each NotebookTab its own respective menu action. However, that would mean storing them all and updating them when the layout is (un)locked.

Another way I suppose would be to instead to re-use the menu action created it Notebook and add it to the NotebookTabs.

Would be a QoL to add the option to the NotebookTabs allowing users to (un)lock the layout with ease rather than clicking the on the blank space in the header. Additionally, it would follow inline with the existing ShowTabs (Toggle Visibility of Tabs) menu action being added to the blank space in the header and every NotebookTab. On the other hand, not doing so would reduce the menu action clutter if we just keep it only in Notebook/the blank space in addition to not much more work of course.

@Felanbird
Copy link
Collaborator

Would be a QoL to add the option to the NotebookTabs allowing users to (un)lock the layout with ease rather than clicking the on the blank space in the header. Additionally, it would follow inline with the existing ShowTabs (Toggle Visibility of Tabs) menu action being added to the blank space in the header and every NotebookTab. On the other hand, would reduce the menu action clutter if we just keep it only in Notebook/the blank space.

This is the important discussion, as visibility of this feature is the worry against it - the more visible it is the more likely somebody will accidentally click it.
From my experience with toggle visibility of tabs, users who have accidentally pressed it do not think to click in the empty spot where their channels used to be. My immediate assumption would be that this pattern would continue, especially if your tabs are still there, thinking to click the small empty area would be overlooked.

@kornes
Copy link
Contributor

kornes commented Mar 24, 2022

useful feature and agree with Felanbird that its risky because toggling it by mistake will frustrate unaware users. There needs to be some kind of feedback for user, showing why tabs cant be reorganized (would also add lock for tab closing).
My suggestion is to remove it from menus and add small custom button with 🔓 icon next to current ➕ for new tabs. Icon could change to open/closed state on click and blink when user tries to do forbidden actions (move, close tab) while locked.

@Infinitay
Copy link
Contributor Author

Personally I'm conflicted on adding a button as a tab button for a visual indication. It would be nice, and I suppose in most cases it is more than fine. However, it would be annoying to have a special case where only the additional button would cause a new line where currently it wouldn't.

image

Or I suppose the true behavior is that the new te tab would be moved down on the new line and the + and 🔒 buttons next to it to fit all three together.

Although again, this is my only complaint with it and thought I should bring it up despite it probably not being the case for the majority of Chatterino users. Personally I rename and reorganize my tabs to make sure theres not just one or two tabs on a new line to free some space for chat viewability.

src/widgets/Notebook.cpp Outdated Show resolved Hide resolved
@Felanbird Felanbird requested a review from pajlada March 26, 2022 09:39
Copy link
Member

@pajlada pajlada left a comment

Choose a reason for hiding this comment

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

👍
Added a commit to fix it not saving in settings
Added the menu item to the notebook tabs - I think it's worth it (see felanbird's comment)
Did some minor refactoring

@Felanbird
Copy link
Collaborator

Also I'll accept that visibility of tabs casing change as a separate PR :)

@pajlada pajlada merged commit 554313d into Chatterino:master Mar 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants