Opened 2 years ago

Closed 2 years ago

Last modified 2 years ago

#13496 closed bug (fixed)

The blue border for File Panel focus does not update correctly

Reported by: owenca Owned by: nobody
Priority: high Milestone: R1/beta1
Component: Kits/libtracker.so Version: R1/Development
Keywords: Tracker, BFilePanel Cc: axeld, waddlesplash, looncraz
Blocked By: Blocking:
Has a Patch: yes Platform: All

Description

Press the Tab key to move the focus to the Cancel button: the blue border enclosing the file list does not disappear completely.

Now click another window to deactivate the file panel and then click the file panel to activate it again: the Cancel button still has the focus, but the file list is enclosed by the blue border as if it had the focus, too.

Please see the attached screenshots.

might be related to #12923

Attachments (8)

Expander 1.png (47.5 KB) - added by owenca 2 years ago.
Expander 2.png (51.0 KB) - added by owenca 2 years ago.
Save as.png (29.2 KB) - added by owenca 2 years ago.
Expander now.png (24.8 KB) - added by owenca 2 years ago.
Save as now.png (26.7 KB) - added by owenca 2 years ago.
Expander new.png (24.8 KB) - added by owenca 2 years ago.
Save as after FilePanelSettings removed.png (26.9 KB) - added by owenca 2 years ago.
0001-Tracker-Fix-blue-border-for-focus-in-file-panel.patch (2.0 KB) - added by owenca 2 years ago.

Download all attachments as: .zip

Change History (27)

Changed 2 years ago by owenca

Attachment: Expander 1.png added

Changed 2 years ago by owenca

Attachment: Expander 2.png added

comment:2 Changed 2 years ago by owenca

comment:3 Changed 2 years ago by owenca

Also, the Save File Panel has two blue bordered boxes (file list and file name) when first activated. Please see the attached screenshot.

Changed 2 years ago by owenca

Attachment: Save as.png added

comment:4 Changed 2 years ago by looncraz

Thanks for finding this.

This is easy to reproduce, but I can't see, yet, why it's happening.

BPoseView::MakeFocus() calls BorderedView::PoseViewFocused() whose logic works fine everywhere else (and is simple enough).

My only thought is that it is somehow not being called when focus is being lost when the view is part of the TFilePanel BWindow. I will continue to investigate this.

comment:5 Changed 2 years ago by owenca

Has a Patch: set

comment:6 Changed 2 years ago by owenca

Fixed! Please see the attached patch.

Changed 2 years ago by owenca

Attachment: Expander now.png added

Changed 2 years ago by owenca

Attachment: Save as now.png added

Changed 2 years ago by owenca

Attachment: Expander new.png added

comment:7 Changed 2 years ago by owenca

Please see the attached screenshots after the fix.

comment:8 Changed 2 years ago by looncraz

Awesome! I was just refreshing to the latest hrev and you have a patch :p

BContainerWindow* window = dynamic_cast<BContainerWindow*>(Window());

If you're going to use dynamic_cast you should probably check for NULL. In this case, though, I'd think that static_cast would be more appropriate.

Also, the "Save as now" seems to be missing the border entirely, which might just be the color choice (probably changing color_which base = B_DOCUMENT_BACKGROUND_COLOR to B_PANEL_BACKGROUND_COLOR on line 4418 of ContainerWindow.cpp will resolve that).

comment:9 in reply to:  8 ; Changed 2 years ago by owenca

Replying to looncraz:

BContainerWindow* window = dynamic_cast<BContainerWindow*>(Window());

If you're going to use dynamic_cast you should probably check for NULL. In this case, though, I'd think that static_cast would be more appropriate.

I copied the code from BorderedView::PoseViewFocused() at line 4416 but forgot to copy the two lines below it that check for NULL. I will fix it. Should I still use dynamic_cast just to be consistent?

Also, the "Save as now" seems to be missing the border entirely, which might just be the color choice (probably changing color_which base = B_DOCUMENT_BACKGROUND_COLOR to B_PANEL_BACKGROUND_COLOR on line 4418 of ContainerWindow.cpp will resolve that).

I just tested it a little more, and this is what happens:

  • Remove ~/config/settings/Tracker/FilePanelSettings.
  • Run Pe and open the Save as... panel. The border is fine. See the attached screenshot "Save as after FilePanelSettings removed" below.
  • Now quit Pe and then run it again. The border in the Save as... panel is messed up as shown in the previously attached screenshot "Save as now".

This only occurs in Save file panel. The Open file panel has no problem.

Changed 2 years ago by owenca

comment:10 in reply to:  9 ; Changed 2 years ago by looncraz

Replying to owenca:

I copied the code from BorderedView::PoseViewFocused() at line 4416 but forgot to copy the two lines below it that check for NULL. I will fix it. Should I still use dynamic_cast just to be consistent?

I think that would be the way to go, it's just a minor run time penalty (a handful of cycles) in any event. If someone wants to improve efficiency, let them do that separately ;-)

I just tested it a little more, and this is what happens:

  • Remove ~/config/settings/Tracker/FilePanelSettings.
  • Run Pe and open the Save as... panel. The border is fine. See the attached screenshot "Save as after FilePanelSettings removed" below.
  • Now quit Pe and then run it again. The border in the Save as... panel is messed up as shown in the previously attached screenshot "Save as now".

This only occurs in Save file panel. The Open file panel has no problem.

Very strange. I will have to run some tests when you post an updated patch to wrap my mind around what you're describing.

comment:11 in reply to:  10 Changed 2 years ago by owenca

Very strange. I will have to run some tests when you post an updated patch to wrap my mind around what you're describing.

Do you want me to include in my next patch your suggestion of "changing color_which base = B_DOCUMENT_BACKGROUND_COLOR to B_PANEL_BACKGROUND_COLOR on line 4418 of ContainerWindow.cpp"?

comment:12 Changed 2 years ago by owenca

New patch attached, with B_DOCUMENT_BACKGROUND_COLOR changed to B_DOCUMENT_BACKGROUND_COLOR.

The border problem with Save file panel is still there.

comment:13 in reply to:  9 ; Changed 2 years ago by humdinger

Replying to owenca:

I just tested it a little more, and this is what happens:

  • Remove ~/config/settings/Tracker/FilePanelSettings.
  • Run Pe and open the Save as... panel. The border is fine. See the attached screenshot "Save as after FilePanelSettings removed" below.
  • Now quit Pe and then run it again. The border in the Save as... panel is messed up as shown in the previously attached screenshot "Save as now".

Sounds like what Giovanni reported in #13497.

comment:14 in reply to:  13 ; Changed 2 years ago by owenca

The broken border of the Save panel was in hrev51166 already, so it's not caused by the patch.

comment:15 Changed 2 years ago by owenca

Please see the latest patch above, with B_DOCUMENT_BACKGROUND_COLOR unchanged.

comment:16 in reply to:  14 Changed 2 years ago by owenca

Replying to owenca:

The broken border of the Save panel was in hrev51166 already, so it's not caused by the patch.

Also, it's likely that only Pe has this problem. StyleEdit is fine.

comment:17 in reply to:  14 Changed 2 years ago by owenca

Replying to owenca:

The broken border of the Save panel was in hrev51166 already, so it's not caused by the patch.

Created a ticket #13521

comment:18 Changed 2 years ago by owenca

looncraz: apply the patch and close this ticket?

comment:19 Changed 2 years ago by waddlesplash

Resolution: fixed
Status: newclosed

Applied in hrev51174. Thanks!

comment:20 in reply to:  18 Changed 2 years ago by looncraz

Replying to owenca:

looncraz: apply the patch and close this ticket?

Looks like waddlesplash beat me to it :p

Note: See TracTickets for help on using tickets.