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.

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

library: Add Header Bar entry #260

Merged
merged 6 commits into from May 30, 2023

Conversation

AkshayWarrier
Copy link
Contributor

Adds Header Bar entry.
Issue workbenchdev/demos#3

@andyholmes andyholmes self-assigned this Apr 10, 2023
Copy link
Contributor

@andyholmes andyholmes left a comment

Choose a reason for hiding this comment

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

Sorry it took me so long to get to this!

This looks really great, I don't think there's much to nitpick about here 🙂

src/Library/demos/Header Bar/main.blp Outdated Show resolved Hide resolved
src/Library/demos/Header Bar/main.blp Outdated Show resolved Hide resolved
Co-authored-by: Andy Holmes <1265208+andyholmes@users.noreply.github.com>
@AkshayWarrier
Copy link
Contributor Author

No worries at all :)

@andyholmes andyholmes self-requested a review April 18, 2023 19:47
Copy link
Contributor

@andyholmes andyholmes left a comment

Choose a reason for hiding this comment

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

LGTM! We can merge if @sonnyp agrees

@sonnyp sonnyp force-pushed the main branch 3 times, most recently from 05f69f1 to 55e450f Compare May 21, 2023 10:37
@sonnyp
Copy link
Contributor

sonnyp commented May 28, 2023

This demonstrates how to use the objects/properties but with Workbench we're also trying to encourage developers on how to use them properly.

Whatever people copy paste from Workbench should be correct and in accordance with the HIG / GNOME practices.

I think there are a couple of issues with this entry:

  1. The main menu should use the open-menu-symbolic icon, have primary: true and "Main Menu" tooltip
  2. some properties don't do anything
  3. we don't want to customize decoration layout, developers should use the default

However if we fix all of these, the entry becomes over-simplified. What might be interesting is to show how header bars can be used in different ways depending on content and type of window.

@bertob wdyt?

@sonnyp
Copy link
Contributor

sonnyp commented May 28, 2023

Ok so I talked with Tobias.
Our suggestion is to replicate this one https://developer.gnome.org/hig/patterns/containers/header-bars.html#button-style (with my comments).

And since other library entries will feature hearbars such as the existing Window and and future ToolbarView entries we will make sure that searching for "header bar" shows all the relevant entries.

@sonnyp sonnyp mentioned this pull request May 28, 2023
@AkshayWarrier AkshayWarrier self-assigned this May 28, 2023
@AkshayWarrier AkshayWarrier requested a review from sonnyp May 30, 2023 14:03
@AkshayWarrier
Copy link
Contributor Author

I could not find some of those icons from that example in the library so I chose to keep the previous ones. I made the other changes you requested.

@sonnyp sonnyp merged commit f440515 into workbenchdev:main May 30, 2023
sonnyp pushed a commit to krlade/Workbench that referenced this pull request Jun 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants