gitify-app / gitify

GitHub notifications on your menu bar. Available on macOS, Windows & Linux.
https://gitify.io
MIT License
4.49k stars 257 forks source link

UI issue after notification is marked as read #1412

Closed akellbl4 closed 2 months ago

akellbl4 commented 2 months ago

🔍 Is there already an issue for your problem?

📝 Description

After clicking on a "read" button (open letter icon) UI becomes broken. I would assume that while the action an error occurred and animation with the following UI updates wasn't performed. Unfortunately, the right mouse click is disabled and I'm not able to check the dev tools console. Let me know if there is a way to open the console, I'll try to leave the app in this state and disable the network for it. Unless it updates the UI for any other reason.

Gitify version: v5.11.0

🪜 Steps To Reproduce

  1. Open app
  2. Click "read" button (open letter icon) on the last notification left.

Gitify Version

Other

Operating System

macOS

GitHub Account

GitHub Enterprise Server

📸 Screenshots

image
setchy commented 2 months ago

Unfortunately, the right mouse click is disabled and I'm not able to check the dev tools console.

You can right-click on the menubar icon and select Toggle Developer Tools, or use the shortcut Cmd + Opt + I

setchy commented 2 months ago

From the screenshot it appears that it's decided to horizontally scroll 🤔 🙃

akellbl4 commented 2 months ago

I didn't provide the best screenshot but I got the app in that state again.

image image

While animation the horizontal scroll appears, and if an error occurs on a request it probably throws before the block is removed so UI gets stuck in that state.

A fix with scroll should be pretty easy to do with overflow-hidden on the container. For the error on the request probably as easy as catching the error so it doesn't stop execution and the block is removed.

As a better solution maybe a loading state should appear on the notification and only when a request is successfully resolved it can be removed from the UI. In this case, it's not an optimistic update, but when we don't control API and they may appear frequently I'd better see a loading state rather than a notification sliding back and forth. Anyway, I leave it at your regard to decide which approach is better for the app :)

akellbl4 commented 2 months ago

After I close developer tools with the error in the console, I can't open them again 😂

UPD: toggling app visibility fixes it

akellbl4 commented 2 months ago

I haven't got time to set up the project locally but I played with it in the dev tools. It's not as easy to fix as I thought, but my solution came to the next changes:

image
setchy commented 2 months ago

I haven't got time to set up the project locally but I played with it in the dev tools. It's not as easy to fix as I thought, but my solution came to the next changes:

* make sidebar and notifications display side by side with flex instead of fixed sidebar

* disable the scroll on the wrapping container of the sidebar and notifications and make it overflow hidden

* overflow is hidden on the notifications container

* add scrollable wrapper inside the notification container
image

thanks @akellbl4 for sharing.

the project is quite simple to setup locally and we'd welcome contributions 🙏

julienperrault commented 2 months ago

I've tried to fix the problem, but it seems impossible because the enterprise api doesn't provide the mark-as-done route: https://docs.github.com/en/enterprise-server@3.12/rest/activity/notifications?apiVersion=2022-11-28#mark-a-thread-as-read Compared to the public endpoint: https://docs.github.com/en/rest/activity/notifications?apiVersion=2022-11-28#mark-a-thread-as-done

setchy commented 2 months ago

I've tried to fix the problem, but it seems impossible because the enterprise api doesn't provide the mark-as-done route: https://docs.github.com/en/enterprise-server@3.12/rest/activity/notifications?apiVersion=2022-11-28#mark-a-thread-as-read Compared to the public endpoint: https://docs.github.com/en/rest/activity/notifications?apiVersion=2022-11-28#mark-a-thread-as-done

@julienperrault - "Mark as Done" was added to GitHub Enterprise Server in version 3.13. See https://docs.github.com/en/enterprise-server@3.13/rest/activity/notifications?apiVersion=2022-11-28#mark-a-thread-as-done

akellbl4 commented 2 months ago

@setchy thanks for linking the docs, I'm going to set up the project locally and contribute as I can ;)

It can be version-related and it might be valuable to show/hide features depending on the API version.