Skip to content

Conversation

@SoNiC-HeRE
Copy link
Contributor

Closes #408

@SoNiC-HeRE SoNiC-HeRE marked this pull request as draft August 11, 2023 18:44
@SoNiC-HeRE
Copy link
Contributor Author

SoNiC-HeRE commented Aug 15, 2023

Kooha-2023-08-15-20-13-43.webm

P.S: the popovermenu bar doesn't seem to work somehow and gives a warning Don't know how to handle this item

@SoNiC-HeRE SoNiC-HeRE marked this pull request as ready for review August 15, 2023 14:45
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.

This one needs a fair amount of cleanup to the UI.

The first thing I would do is move the boxed list and reveal buttons into the Adw.ToolbarView. The list rows labels also need to be updated to use labels following the HIG. Also it doesn't seem like any of the tab or view switchers work.

I'm thinking this one might need to be in a proper window, because the borders don't look quite right, and the close buttons actually try to close the Workbench window.

@andyholmes andyholmes self-assigned this Aug 16, 2023
@SoNiC-HeRE SoNiC-HeRE requested a review from andyholmes August 16, 2023 13:30
Copy link
Contributor

@sonnyp sonnyp left a comment

Choose a reason for hiding this comment

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

Feedback from alice

why does the header bar have a GtkLabel in it?

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.

More notes 🙂

label:_("Header Bar");
};
}
content: Adw.StatusPage {
Copy link
Contributor

Choose a reason for hiding this comment

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

Switch this to an Adw.ViewStack

Copy link
Contributor

Choose a reason for hiding this comment

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

Why? there is nothing to switch to

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, I'm confused now. I thought you wanted to demonstrate all the different bars? Unless I'm mistaken you need an Adw.ViewStack for the Adw.ViewSwitcherBar to work?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I guess the idea is to have that other view switcher in the .blp file, along with the other widget? I guess I'm confused why not just actually use a view switcher?

Copy link
Contributor Author

@SoNiC-HeRE SoNiC-HeRE Aug 17, 2023

Choose a reason for hiding this comment

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

Kooha-2023-08-17-18-11-54.webm

@sonnyp @andyholmes Is this the kind of UI we wanted to achieve?

P.S: Yes, I'll get rid of those irrelevant viewstacks like account and notifications

@SoNiC-HeRE
Copy link
Contributor Author

Applied suggested changes

  • Implemented Adw.ViewStack
  • Updated Row titles
  • Used Adw.PreferencesPage
  • Made ToggleButtons active by default
  • Refactored Code

New issues:

  • A new warning (re.sonny.Workbench:4): Gtk-CRITICAL **: 21:27:57.008: GtkBox 0x5559cd41aa80 reports a minimum width of 15, but minimum width for height of 1048576 is 24. Expect overlapping widgets. seems some sort of spacing issue.
  • Also except for the Header Bar i can't simultaneously choose 2 same widgets at both top and bottom bar.
    For example:
    If i want 2 action bars both at top and bottom.I'm unable to one of them doesn't appear.

@SoNiC-HeRE SoNiC-HeRE requested a review from andyholmes August 17, 2023 16:04
Diego-Ivan and others added 8 commits August 18, 2023 09:51
@sonnyp sonnyp self-requested a review August 18, 2023 09:28
@sonnyp sonnyp assigned sonnyp and unassigned andyholmes Aug 18, 2023
@sonnyp sonnyp removed the request for review from andyholmes August 18, 2023 09:29
@sonnyp sonnyp merged commit 8ed5a8c into main Aug 20, 2023
@sonnyp sonnyp deleted the sonic/toolbarview branch August 20, 2023 10:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add ToolbarView entry

8 participants