Add top banner #2381
Add top banner #2381
Conversation
| --banner-background-color-warning: #daad1b; | ||
| --banner-background-color-success: #67da1b; | ||
| --banner-background-color-plain: var(--more-accented-background-color); | ||
| --banner-color: var(--main-text-color); |
There was a problem hiding this comment.
I don't think it's needed to have this themable since it's for exceptional cases only and user shouldn't see it often.
src/public/stylesheets/style.css
Outdated
| @@ -55,10 +55,6 @@ table td, table th { | |||
| background-color: var(--modal-backdrop-color) !important; | |||
| } | |||
|
|
|||
| .component { | |||
| contain: size; | |||
| } | |||
There was a problem hiding this comment.
This PR shouldn't be changing such fundamental classes.
| @@ -10,4 +10,10 @@ export default class FlexContainer extends Container { | |||
|
|
|||
| this.attrs.style = `display: flex; flex-direction: ${direction};`; | |||
| } | |||
|
|
|||
| withFullSize() { | |||
| this.attrs.style += `width: 100%; height: 100%;`; | |||
There was a problem hiding this comment.
This does not look correct.
The root container is already 100% height, banner has certain fixed height and then the rest (the main app) should be filled with flex-grow (filling() method)
Alternatively, wouldn't it be better to have this floating with absolute coordinates above (in the z-index sense) the main content? Since with your approach the main app will jump up and down as connection is lost/renewed.
In such case bootstrap toasts might be used as an existing solution.
There was a problem hiding this comment.
Alternatively, wouldn't it be better to have this floating with absolute coordinates above (in the z-index sense) the main content? Since with your approach the main app will jump up and down as connection is lost/renewed.
I don't think this is a good idea. This is what it looks like:
There was a problem hiding this comment.
What about this:
can be done with few lines of code:
toastService.showPersistent({
id: "network-down",
title: "Network is down!",
message: "Trilium backend is currently not accessible.",
icon: "wifi-off"
})There was a problem hiding this comment.
That's intrusive. You can't see the full title anymoe and some buttons are harder to reach. If you're on a plane, editing your notes with only partial internet, this would drive you crazy since you wouldn't be able to do your workflow properly.
There was a problem hiding this comment.
When you're on a plane and don't have internet, basically no functionality will work correctly. You should in fact stop editing until you have an internet connection, otherwise you risk data loss.
There was a problem hiding this comment.
Wait - just for clarification. Does Trilium Notes save the progress offline and then uploads it once it has a connection back?
There was a problem hiding this comment.
Web version - no, it has no offline storage.
Desktop version works fully offline and then synchronizes with the server periodically.
There was a problem hiding this comment.
Okay in that case your solution would fit as well for the web version (only). However, we should probably change the location to the top right (or something less intrusive).
| this.$timer = this.$widget.find(".timer"); | ||
|
|
||
| this.$widget.on("mouseenter", this.pauseTimer.bind(this)); | ||
| this.$widget.on("mouseleave", this.resumeTimer.bind(this)); |
There was a problem hiding this comment.
Can you clarify what's going on here?
In my mind it's really simple - (non-pausable) timer, when it detects that the network is down it will show the banner, once it's up again, banner disappears. But here I see info, warning, success, plain, what is all that for?
There was a problem hiding this comment.
In case we want to show other notifications, they should be pauseable
There was a problem hiding this comment.
To be honest I don't understand what "pausable notification" is, but in general YAGNI principle applies here.
|
@Myzel394 I do really think to have a place for showing the connect status and sync status for WEB version is a great idea. |
closes #2366
This is not the full PR yet. @zadam what do you think so far? If you are ok with this approach so far, I will add the internet-lost-message.