8000 Fix pressing 'Close' on persistent tabs not working. by HalfVoxel · Pull Request #790 · material-shell/material-shell · GitHub
[go: up one dir, main page]
Skip to content

Fix pressing 'Close' on persistent tabs not working.#790

Open
HalfVoxel wants to merge 1 commit intomainfrom
aron/fix_closing_persistent_impossible
Open

Fix pressing 'Close' on persistent tabs not working.#790
HalfVoxel wants to merge 1 commit intomainfrom
aron/fix_closing_persistent_impossible

Conversation

@HalfVoxel
Copy link
Contributor
@HalfVoxel HalfVoxel commented Apr 23, 2022 •
edited
Loading

Description

Previously, right clicking on a persistent tab and pressing the Close option would just close the app, but not remove the tab. Clicking Close again on a persistent tab which only had a placeholder would do nothing.

This PR fixes this so that it is possible to close persistent tabs.

Checklist:

  • I used indents of four spaces in code and two spaces in documentation
  • I have performed a self-review of my own code, it does not generate any errors
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation or will submit them later
  • I have recompiled gschgema (if there were any changes)
  • Changes to the SASS were done to the variables only or following the proper way

@HalfVoxel HalfVoxel requested a review from PapyElGringo April 23, 2022 20:09
@jntesteves
Copy link
Contributor
8000 jntesteves commented May 16, 2022

If the tab is pinned, the user still needs a way to close the app with the mouse without removing persistence. That's what the Close option does, you can't just remove it without providing a replacement somewhere else, the user needs the option. The Unpin option is there if the user wants to remove that, too.

@PapyElGringo
Copy link
Collaborator
PapyElGringo commented Jun 16, 2022

I do agree with @jntesteves here too

@HalfVoxel
Copy link
Contributor Author
HalfVoxel commented Jun 16, 2022

That's fair. I agree with that point too.
But I still think that if should close the tab if the tile is just a placeholder. Otherwise to close a pinned tab you first have to unpin it and then click close. Right now the close button on such tabs just silently does nothing. Do you agree?

@PapyElGringo
Copy link
Collaborator
PapyElGringo commented Jun 16, 2022

I agree that the useless close button on a placeholder is an issue. We could make it Unpin the placeholder and close it or replace it by Open application

< 700D template class="js-file-alert-template">
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@PapyElGringo PapyElGringo Awaiting requested review from PapyElGringo

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@HalfVoxel @jntesteves @PapyElGringo
0