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

fix(ui): notifications appearing on top in unintended situations #2093

Merged
merged 2 commits into from
Jul 6, 2023
Merged

fix(ui): notifications appearing on top in unintended situations #2093

merged 2 commits into from
Jul 6, 2023

Conversation

Subjective
Copy link
Contributor

This fixes an issue where notifications were obscuring UI elements of plugins like Noice.

@github-actions
Copy link

github-actions bot commented Jul 5, 2023

Use the following updater settings in your user/init.lua file, restart, and run :AstroUpdate to test this pull request:

  updater = {
    channel = "nightly",
    branch = "fix-notify-zindex",
    remote = "Subjective",
    remotes = {
      ["Subjective"] = "Subjective/AstroNvim",
    },
  },

@mehalter
Copy link
Member

mehalter commented Jul 5, 2023

What UI situations would happen in which you would not want the notification to appear on the top? I am not sure I can think of one

@mehalter
Copy link
Member

mehalter commented Jul 5, 2023

A screenshot might be helpful as well showing the unintended behavior

@Subjective
Copy link
Contributor Author

Subjective commented Jul 6, 2023 •
edited
Loading

@mehalter The Noice cmdline. In my opinion it makes more sense to have the cmdline always appear on top, especially when entering cmdline mode after a notification appears.

From what I can tell, setting the zindex 150 is low enough that it notifications appear above the cmdline while remaining underneath mason and lazy.

Before image
After image

@mehalter
Copy link
Member

mehalter commented Jul 6, 2023 •
edited
Loading

@Subjective if you set it to 200 will it show the last one on top? If a notification comes up while the command line is open then that would be over, but if you open the command line with a notification open then it would be on top? I'm not sure how ties are resolved. But I think that might be better behavior out of the box if possible

Edit: I did test this and it doesn't work quite as nicely as I would prefer so we will just make it slightly less

mehalter
mehalter requested changes Jul 6, 2023
View reviewed changes
lua/plugins/ui.lua Outdated Show resolved Hide resolved
@Subjective @mehalter
fix(ui): increase notification zindex to 175
1e6a845 
Co-authored-by: Micah Halter <micah@mehalter.com>
mehalter
mehalter approved these changes Jul 6, 2023
View reviewed changes
Copy link
Member

@mehalter mehalter left a comment

Choose a reason for hiding this comment

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

Awesome! Thanks for taking the time to open a PR to resolve this :)

@mehalter mehalter merged commit a0803b7 into AstroNvim:nightly Jul 6, 2023
4 checks passed
mehalter added a commit that referenced this pull request Jul 6, 2023
@Subjective @mehalter
fix(ui): notifications appearing on top in unintended situations (#2093)
698f39b 
* fix(ui): notifications appearing on top in unintended situations

* fix(ui): increase notification zindex to 175

Co-authored-by: Micah Halter <micah@mehalter.com>

---------

Co-authored-by: Micah Halter <micah@mehalter.com>
mehalter added a commit that referenced this pull request Jul 6, 2023
@Subjective @mehalter
fix(ui): notifications appearing on top in unintended situations (#2093)
bcb9c56 
* fix(ui): notifications appearing on top in unintended situations

* fix(ui): increase notification zindex to 175

Co-authored-by: Micah Halter <micah@mehalter.com>

---------

Co-authored-by: Micah Halter <micah@mehalter.com>
mehalter added a commit that referenced this pull request Jul 6, 2023
@Subjective @mehalter
fix(ui): notifications appearing on top in unintended situations (#2093)
b7d6368 
* fix(ui): notifications appearing on top in unintended situations

* fix(ui): increase notification zindex to 175

Co-authored-by: Micah Halter <micah@mehalter.com>

---------

Co-authored-by: Micah Halter <micah@mehalter.com>
mehalter added a commit that referenced this pull request Jul 6, 2023
@Subjective @mehalter
fix(ui): notifications appearing on top in unintended situations (#2093)
9eefade 
* fix(ui): notifications appearing on top in unintended situations

* fix(ui): increase notification zindex to 175

Co-authored-by: Micah Halter <micah@mehalter.com>

---------

Co-authored-by: Micah Halter <micah@mehalter.com>
mehalter added a commit that referenced this pull request Jul 6, 2023
@mehalter @Subjective @TiagoAraujoDev
v3.31.0 Nightly (#2091)
467a270 
* fix(utils): load treesitter when making notifications

* fix(treesitter): ensure markdown treesitter parser

* feat: add `vim.g.git_worktrees` to enable usage of detached git working trees (#2092)

* feat(options): add setting to enable git integration for custom worktrees

* fix(autocmds): add additional check for AstroGitFile to see if file is in custom worktree

* refactor(autocmds): extract `in_worktree()` to custom utility function `find_worktree()`

* feat(mappings): allow toggle lazygit to automatically load git worktrees if available

* chore(mappings): cleanup redundant toggle lazygit mapping

* refactor: clean up implementation of git worktrees

* fix(ui): notifications appearing on top in unintended situations (#2093)

* fix(ui): notifications appearing on top in unintended situations

* fix(ui): increase notification zindex to 175

Co-authored-by: Micah Halter <micah@mehalter.com>

---------

Co-authored-by: Micah Halter <micah@mehalter.com>

* feat: include tables for all map modes in `mappings` and `lsp.mappings`

* chore: use `vim.ui.open` if it's available

* fix(utils): set cursor in a better position (#2094)

* fix: disable `lua_ls` formatting with Neoconf

* fix(lsp): hacky fix for neoconf lazy loading

* fix(heirline): improve lualine integration with statusline

* refactor: simplify notification formatting implementation

* fix(utils): update `is_available` to use the correct lazy API

* chore(snapshot): update lazy_snapshot

---------

Co-authored-by: Josh <56745535+Subjective@users.noreply.github.com>
Co-authored-by: Tiago Muniz de Araujo <tiagomuniz130@gmail.com>
mehalter added a commit that referenced this pull request Jul 6, 2023
@Subjective @mehalter
fix(ui): notifications appearing on top in unintended situations (#2093)
a918f87 
* fix(ui): notifications appearing on top in unintended situations

* fix(ui): increase notification zindex to 175

Co-authored-by: Micah Halter <micah@mehalter.com>

---------

Co-authored-by: Micah Halter <micah@mehalter.com>
@Subjective Subjective deleted the fix-notify-zindex branch July 21, 2023 22:00
MattCassar pushed a commit to MattCassar/AstroNvim that referenced this pull request Sep 9, 2023
@Subjective @mehalter @MattCassar
fix(ui): notifications appearing on top in unintended situations (Ast…
f78d615 
…roNvim#2093)

* fix(ui): notifications appearing on top in unintended situations

* fix(ui): increase notification zindex to 175

Co-authored-by: Micah Halter <micah@mehalter.com>

---------

Co-authored-by: Micah Halter <micah@mehalter.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mehalter mehalter mehalter 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.

None yet
2 participants
@Subjective @mehalter