Opened 10 years ago

Closed 7 years ago

#3787 closed enhancement (fixed)

Poorman: Use layoutmanager

Reported by: humdinger Owned by: yourpalal
Priority: normal Milestone: R1
Component: Applications/PoorMan Version: R1/Development
Keywords: Cc: mks@…
Blocked By: Blocking:
Has a Patch: yes Platform: All

Description

This is hrev30234.

There are several issues with cut off labels and buttons. PoorMan could do with some layoutmanaging love. The first-startup alert, too, the last "?" looks lost, alone in the last line.

I felt using "Folder" consistently instead of "Directory" would be better. I also changed the menu item "Preferences" to "Settings" which makes more sense in configuring the server as it's not a question of personal preference. "Index Page" became a more user friendly "Start Page". See attached patch.

Here are a few enhancements, maybe good tasks for newcomers:

  • Have the content of the console resize with the window. Same goes for the settings panel.
  • A bit more space between the console and the status info above it.
  • The location settings text boxes don't accept drag&drops (how un-Haiku).
  • Combine location of Web Folder and Start Page. Since the start page has to be inside the web folder, open just one file panel. If the user selects a folder, only fill in the Web Folder location, if he selects a file, choose that file as Start Page and its folder as Web Folder.

There's no category Applications | PoorMan yet. [Also, what have you done to Trac? It's lightning fast right now!]

Attachments (3)

PoorMan.diff (1.9 KB) - added by humdinger 10 years ago.
poorman.diff (68.2 KB) - added by tokyo6pm 10 years ago.
layout manager support and coding style cleanup
poorman.2.diff (65.8 KB) - added by mks 7 years ago.
Updated patch against hrev44240

Download all attachments as: .zip

Change History (17)

Changed 10 years ago by humdinger

Attachment: PoorMan.diff added

comment:1 Changed 10 years ago by humdinger

Applied patch with hrev30433.

Changed 10 years ago by tokyo6pm

Attachment: poorman.diff added

layout manager support and coding style cleanup

comment:2 Changed 10 years ago by stpere

Owner: changed from axeld to stpere
Status: newassigned

Hi tokyo6pm,

I made some changes to PoorMan in hrev32486. I didn't saw your patch before :( Well, I will try to incorporate your changes with mine!

comment:3 Changed 10 years ago by umccullough

Ah bummer!

Wonder when we'll get the Trac attachment notifications ;)

comment:4 Changed 9 years ago by mmadia

Has a Patch: set

comment:5 Changed 9 years ago by mmadia

Component: ApplicationsApplications/Poorman
Owner: changed from stpere to yourpalal
Status: in-progressassigned
Version: R1/pre-alpha1R1/Development

comment:6 Changed 7 years ago by leavengood

The patch looks quite good, not only adding the layout management but fixing coding violations. Maybe most of it will still apply? At least it is a task in GCI now, and I posted a comment on the GCI task to indicate there is already a patch. Maybe a student can try to apply and update the patch.

Either way tokyo6pm should get the credit for his work, and have it applied in Haiku. If no GCI student gets around to it, I'll try to do it.

comment:7 in reply to:  6 Changed 7 years ago by yourpalal

Replying to leavengood:

The patch looks quite good, not only adding the layout management but fixing coding violations. Maybe most of it will still apply? At least it is a task in GCI now, and I posted a comment on the GCI task to indicate there is already a patch. Maybe a student can try to apply and update the patch.

Ah, yeah, I didn't realize there was a patch here. Your comment doesn't show up in Melange, I guess it got lost somewhere along the way. I think having a student apply and update the patch is good. The layout code uses the old BGroupLayoutBuilder, when it should be using BLayoutBuilder::Group<>, so the student could fix that as well.

Either way tokyo6pm should get the credit for his work, and have it applied in Haiku. If no GCI student gets around to it, I'll try to do it.

Yeah, totally, it's a shame that we missed this one, I imagine tokyo6pm was discouraged, and his/her work looks good.

Changed 7 years ago by mks

Attachment: poorman.2.diff added

Updated patch against hrev44240

comment:8 Changed 7 years ago by mks

I updated tokyo6pm's patch against current master. I can understand that he was never heard of again, as I guess this was a rather big pile of work and manually reapplying it wasn't the most pleasant thing either. ;)

There's one problem, however, I could not solve: The preferences dialog doesn't really work on my machine. The text boxes and labels don't show up inside the BBoxes. I couldn't figure out, what's wrong. Could anybody else please have a look at this?

comment:9 Changed 7 years ago by mks

Cc: mks@… added

comment:10 Changed 7 years ago by yourpalal

Hi mks, thanks for working on this ticket.

Some comments on layout building:

  • Please switch from using BGroupLayoutBuilder (GroupLayoutBuilder.h) to BLayoutBuilder::Group (LayoutBuilder.h) BGroupLayoutBuilder will be deprecated soon, so we don't want to introduce any more code that uses it into our codebase. This isn't really documented anywhere, so I can't blame you for not knowing it! Thankfully the interfaces are very similar, and it's quite fast to switch from one to the other.
  • Once you've made the switch, instead of doing Add(BGroupLayoutBuilder() ... ), please use the AddGroup() method, which is very similar.
  • Instead of someBBox->AddChild(BGroupLayoutBuilder(..)... you can use BLayoutBuilder::Group<>(someBBox, ...) to build a group layout in the bbox.
  • You can also use the BLayouBuilder::Group<>::AddGroup() methods in a similar way to add a view with a group layout
  • Instead of setting the layout on a view/window and then adding a GroupView in the view/window, it is preferred to build the layout directly in the view/window, just start with a BLayoutBuilder::Group<>(view_or_window, ...)
  • For some documentation of the BLayoutBuilder classes, including a short tutorial on using them, you can see http://api.haiku-os.org/layout_intro.html and the docs it links to.

I appreciate the style fixes, but the style for inline methods is different from what you used in PoorManLoggingView.h . For instance, SetLogConsoleValue should look like this:

        void SetLogConsoleValue(bool state) {
             if (state)
                 fLogConsole->SetValue(B_CONTROL_ON);
             else
                 fLogConsole->SetValue(B_CONTROL_OFF);
        }

misc:

  • You included SpaceLayoutItem.h in PoorManPreferencesWindow but didn't use any classes from it.
  • In the future, if there are that many style issues, it is better to fix them in a separate patch, so that the functional changes are easier to spot. I know this was inherited from tokyo6pm, but it's something to keep in mind for your future endeavours :)

I didn't apply/compile/test the patch, but try making these changes. If you are still having issues with the BBoxes once that's done, I can hopefully look deeper into the problem sometime in the future.

Thanks again for your work :)

comment:11 Changed 7 years ago by mks

Just to make this clear again: This patch is not my work at all. I merely re-applied it to current master. ;)

I can try to apply your comments, though.

comment:12 Changed 7 years ago by mks

Just FYI … I have a hard time getting motivated on doing this. I mainly did this, as I saw that the original patch was not included because it was forgotten and orphaned and didn't apply cleanly anymore and also because people who said, they would work on it didn't. However as it's not my patch and I don't see the big benefit, I am not motivated at all to fix the stuff yourpalal mentioned. I also have not enough experience to fix all of it and as no one else seems to be interested in working on this, it's going to stay unfinished anyways.

So - sorry if the patch is not ready for inclusion, but I will not do it.

comment:13 in reply to:  12 Changed 7 years ago by yourpalal

Replying to mks:

So - sorry if the patch is not ready for inclusion, but I will not do it.

That's okay! The work you did was still helpful! Maybe I can get around to polishing and committing these changes in the near future (before it gets outdated once more!).

comment:14 Changed 7 years ago by yourpalal

Resolution: fixed
Status: assignedclosed

Applied & committed this patch, along with some fixes of my own in hrev44346. Thanks for your work tokyo6pm and mks!

Note: See TracTickets for help on using tickets.