Skip to content

fix: Configurable essentials max fix conflict of PR 8441 #9189

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

Open
wants to merge 20 commits into
base: dev
Choose a base branch
from

Conversation

MiraiDevv
Copy link

resolve conflict of PR: #8441

@dosubot dosubot bot added the size:L This PR changes 100-499 lines, ignoring generated files. label Jun 26, 2025
@mr-cheffy
Copy link
Contributor

Should we close #8441 ?

@MiraiDevv
Copy link
Author

Should we close #8441 ?

I think so, I think the author hasn't seen it yet and it will probably take a while to see it, I decided to go ahead a little to see if you would accept the PR, but the author of this PR that I marked is not me, we are different people, just to be clear.

@MiraiDevv
Copy link
Author

Before merging this PR, I had one thought, what do you think about us establishing certain confirmation messages (pop-ups) to be opened, when certain amounts of essential tabs are changed? so for example, 4 essential tabs open => generate a warning that if you have 8gb of ram or less this may affect performance, 8 essential tabs open => generate a warning that if you have 16gb of ram or less this may affect browser performance and consume a lot of RAM (and this could considerably reduce most of the issues that people end up opening here on github, because if they read it, they will have an idea of ​​what is happening, because the warning was displayed), remembering that these warnings about the number of tabs and amount of ram are merely hypothetical, no calculation was made about this, this would just be a way to avoid many issues being opened for a problem that would be easily solved just by removing some essential tabs, anyway, please give your opinion on this and what you think about it, if this has already been discussed or not in the previous PR, I did see something like this being discussed, but not together with this new feature.

name = 'Space',
icon = undefined,
dontChange = false,
name,

Choose a reason for hiding this comment

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

I don't recognize the changes from here down

The name value probably needs to have an initial value, so I put the default value back, but the icon, from what I understand from Zen and also from JavaScript, does not need to have an undefined value, since by default, without receiving any value, it is already undefined.

Signed-off-by: Mirai <mirai.dev@outlook.com>
…ding essential container updates and improved handling of extra tabs, b=no-bug, c=workspaces
@MiraiDevv
Copy link
Author

I realized that I had removed some things that were important, I put them back looking at the code that was there before, if anyone has any more changes that need to be made or thinks that something is different, you can let me know, please, have a good day.

@mr-cheffy
Copy link
Contributor

This should be without a second pref for height, it should be dynamic

@MiraiDevv
Copy link
Author

This should be without a second pref for height, it should be dynamic

Is that what you meant? From what I understood, it was just to remove the logic that was determining the height, if not, if you can point to the line/file or explain more comprehensively, if it's not too much trouble.

…tab management (it had another identical method above).
@mr-cheffy
Copy link
Contributor

That's exactly what I mean, why is ZenWorkspaces.mjs a new created file though?

@MiraiDevv
Copy link
Author

That's exactly what I mean, why is ZenWorkspaces.mjs a new created file though?

I didn't understand either, I just remember that I accidentally removed some lines and then put them back, the last time I compared the two files they were the same, I won't be sure just by saying, but I think it was a github bug, because I didn't even delete the file...........

@blakegearin
Copy link

Just wanted to call out this has been awaiting approval for over 60 days now (factoring in both PRs)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size:L This PR changes 100-499 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants