-
-
Notifications
You must be signed in to change notification settings - Fork 90
Library: Add Banner Entry #243
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.
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
Conversation
src/Library/demos/Banner/main.js
Outdated
|
|
||
| function adwBanner() { | ||
| // Create a new AdwBanner widget | ||
| const banner = new Adw.Banner({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to #248
please create the banner from the blueprint
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since these are generally intended to be place in a window, directly under the header bar, it would be a good idea to create a small window to demonstrate this, too.
The button in Workbench could pop up a window, and contain a few widget to control the info bar's properties.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree with Andy, the Banner should be in a Window.
No need for a custom button to open the window though - Workbench will directly offer the option if it detects a Window in the Blueprint. Similar to the "Window" library entry.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, the libadwaita demo doesn't use an external banner for its demo – I think it's safe to follow them and makes the demo visible immediately.
works for you @andyholmes ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, that's good too. I think the important part is that it doesn't have margins on the top or left/right sides.
Replicating the libadwaita demo is good idea in general I think.
andyholmes
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking better!
Be sure to mark all user-visible strings in the .js and .blp file as translatable, by putting them in the gettext function, like _("Words to translate").
| Adw.Banner banner{ | ||
| button-label: "Troubleshoot"; | ||
| title: "An error occurred: Could not resolve host."; | ||
| revealed: true; | ||
| use-markup: true; | ||
| margin-bottom: 20; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is better, but the banner should really be up at the top of the window, with no margins, right under the header bar.
Then maybe make it so clicking "Troubleshoot" hides the banner and shows the toast. Where the banner is now, add a button so the banner can be re-opened.
| Adw.ToastOverlay overlay { | ||
| Adw.StatusPage { | ||
| title: "Adw Banner"; | ||
| description: "Gnome Banner Test"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| description: "Gnome Banner Test"; | |
| description: "GNOME Banner Test"; |
05f69f1 to
55e450f
Compare
|
hey @Kodecheff, do you want to finish this? If so, make sure to add your name to the list of contributors in about.js |

Fixes #285