bamaer commented on PR #8270: URL: https://github.com/apache/hop/pull/8270#issuecomment-5559321062
Review feedback addressed in the three commits above: correctness and security fixes, the source dialog rewritten onto `@GuiWidgetElement`, and the unread bell registration below. Individual replies are on each thread. Separate from the review points: while testing the above in Hop Web I found that the unread indicator on the notification bell never appears there. It reproduces on this branch before any of the review fixes, so it isn't a regression from them. The cause isn't in the notification code, which is why reading those files doesn't reveal it. `HopWeb.configure()` registers the image each toolbar item *declares*, from the GUI registry. An icon that replaces it later has to be registered separately, or `RWT.getResourceManager().getLocation()` returns the location of a resource that was never registered: the `img src` 404s and the icon silently doesn't change, with no error on the server or in the browser. There's already a list of such alternates in `HopWeb` for the show/hide toggles, with a comment explaining exactly this — `notification-bell-unread.svg` was just never added to it. Fixed by adding it to that list, and extended the comment so the next alternate icon doesn't hit the same thing. Verified in Hop Web: the resource now registers at startup and serves the SVG carrying the dot, and the indicator appears. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
