View Issue Details

IDProjectCategoryView StatusLast Update
0027171LazarusLCLpublic2014-12-28 13:09
ReporterBart BroersmaAssigned ToBart Broersma 
PrioritynormalSeverityminorReproducibilityalways
Status closedResolutionfixed 
Platformi386OSWindowOS VersionWin7
Product Version1.3 (SVN)Product Buildr47075 
Target Version1.2.8Fixed in Version1.2.8 
Summary0027171: TCustomGrids popumenu does not show when editor has focus
DescriptionHewn you assign a popupmenu to a grid, it only popups when you right-click outside an active editor.
Inside the editor the default OS pupmenu appears.

Steps To ReproduceBuild and run attached demo (sg.zip)
Right-click on grid outside an editor: grid's popu appears.
Left-click on a cel to get an active editor, right-click on that editor: OS default popup appears.
Additional InformationIn D7 the grids popupmenu also pops up inside an active editor.
TagsNo tags attached.
Fixed in Revisionr47258
LazTarget1.2.8
Widgetset
Attached Files
  • sg.zip (6,048 bytes)
  • grids.popup.diff (382 bytes)
    Index: lcl/grids.pas
    ===================================================================
    --- lcl/grids.pas	(revision 47075)
    +++ lcl/grids.pas	(working copy)
    @@ -7967,6 +7967,7 @@
       end;
       if aEditor<>Editor then
         Editor := aEditor;
    +  if Assigned(Editor) then Editor.PopupMenu := PopupMenu;
       {$ifdef DbgGrid}
       DebugLnExit('TCustomGrid.SelectEditor END');
       {$endif}
    
    grids.popup.diff (382 bytes)
  • grids.popup.2.diff (4,166 bytes)
    Index: lcl/grids.pas
    ===================================================================
    --- lcl/grids.pas	(revision 47075)
    +++ lcl/grids.pas	(working copy)
    @@ -34,7 +34,7 @@
     uses
       Types, Classes, SysUtils, Math, Maps, LCLStrConsts, LCLProc, LCLType, LCLIntf,
       FileUtil, FPCanvas, Controls, GraphType, Graphics, Forms, DynamicArray,
    -  LMessages, StdCtrls, LResources, MaskEdit, Buttons, Clipbrd, Themes,
    +  LMessages, StdCtrls, LResources, MaskEdit, Buttons, Clipbrd, Themes, Menus,
       LazUtf8Classes, Laz2_XMLCfg; // <-- replaces XMLConf (part of FPC libs)
     
     const
    @@ -675,6 +675,7 @@
         FEditorShowing: Boolean;
         FEditorKey: Boolean;
         FEditorOptions: Integer;
    +    FEditorPopupMenu: TPopupMenu;
         FExtendedSelect: boolean;
         FFastEditing: boolean;
         FAltColorStartNormal: boolean;
    @@ -796,6 +797,7 @@
         function  GetColWidths(Acol: Integer): Integer;
         function  GetColumns: TGridColumns;
         function  GetEditorBorderStyle: TBorderStyle;
    +    function  GetEditorPopupMenu: TPopupMenu;
         function  GetBorderWidth: Integer;
         function  GetRowCount: Integer;
         function  GetRowHeights(Arow: Integer): Integer;
    @@ -834,6 +836,7 @@
         procedure SetDefRowHeight(AValue: Integer);
         procedure SetDefaultDrawing(const AValue: Boolean);
         procedure SetEditor(AValue: TWinControl);
    +    procedure SetEditorPopupMenu(Value: TPopupMenu);
         procedure SetFixedRows(const AValue: Integer);
         procedure SetFocusColor(const AValue: TColor);
         procedure SetGridLineColor(const AValue: TColor);
    @@ -1094,6 +1097,7 @@
         property EditorMode: Boolean read FEditorMode write EditorSetMode;
         property EditorKey: boolean read FEditorKey write FEditorKey;
         property EditorOptions: Integer read FEditorOptions write SetEditorOptions;
    +    property EditorPopupMenu: TPopupMenu read GetEditorPopupMenu write SetEditorPopupMenu;
         property EditorShowing: boolean read FEditorShowing write FEditorShowing;
         property ExtendedColSizing: boolean read FExtendedColSizing write FExtendedColSizing;
         property ExtendedRowSizing: boolean read FExtendedRowSizing write FExtendedRowSizing;
    @@ -1288,6 +1292,7 @@
         property Editor;
         property EditorBorderStyle;
         property EditorMode;
    +    property EditorPopupMenu;
         property ExtendedColSizing;
         property AltColorStartNormal;
         property FastEditing;
    @@ -1425,6 +1430,7 @@
         property DragCursor;
         property DragKind;
         property DragMode;
    +    property EditorPopupMenu;
         property Enabled;
         property ExtendedSelect;
         property FixedColor;
    @@ -1605,6 +1611,7 @@
           property Cols[index: Integer]: TStrings read GetCols write SetCols;
           property DefaultTextStyle;
           property EditorMode;
    +      property EditorPopupMenu;
           property ExtendedSelect;
           property Objects[ACol, ARow: Integer]: TObject read GetObjects write SetObjects;
           property Rows[index: Integer]: TStrings read GetRows write SetRows;
    @@ -1642,6 +1649,7 @@
         property DragCursor;
         property DragKind;
         property DragMode;
    +    property EditorPopupMenu;
         property Enabled;
         property ExtendedSelect;
         property FixedColor;
    @@ -2429,6 +2437,13 @@
       {$endif}
     end;
     
    +procedure TCustomGrid.SetEditorPopupMenu(Value: TPopupMenu);
    +begin
    +  FEditorPopupMenu := Value;
    +  if FEditorPopupMenu <> nil then
    +    FEditorPopupMenu.FreeNotification(Self);
    +end;
    +
     procedure TCustomGrid.SetFixedCols(const AValue: Integer);
     begin
       if FFixedCols=AValue then begin
    @@ -4977,6 +4992,11 @@
         doTopleftChange(False)
     end;
     
    +function TCustomGrid.GetEditorPopupMenu: TPopupMenu;
    +begin
    +  Result := FEditorPopupMenu;
    +end;
    +
     function TCustomGrid.IsCellButtonColumn(ACell: TPoint): boolean;
     var
       Column: TGridColumn;
    @@ -5190,6 +5210,7 @@
       end;
     end;
     
    +
     procedure TCustomGrid.SetTitleFont(const AValue: TFont);
     begin
       FTitleFont.Assign(AValue);
    @@ -7967,6 +7988,8 @@
       end;
       if aEditor<>Editor then
         Editor := aEditor;
    +  if Assigned(Editor) and not Assigned(Editor.PopupMenu) then
    +    Editor.PopupMenu := EditorPopupMenu;
       {$ifdef DbgGrid}
       DebugLnExit('TCustomGrid.SelectEditor END');
       {$endif}
    
    grids.popup.2.diff (4,166 bytes)

Activities

Bart Broersma

2014-12-16 20:48

developer  

sg.zip (6,048 bytes)

Bart Broersma

2014-12-17 11:46

developer  

grids.popup.diff (382 bytes)
Index: lcl/grids.pas
===================================================================
--- lcl/grids.pas	(revision 47075)
+++ lcl/grids.pas	(working copy)
@@ -7967,6 +7967,7 @@
   end;
   if aEditor<>Editor then
     Editor := aEditor;
+  if Assigned(Editor) then Editor.PopupMenu := PopupMenu;
   {$ifdef DbgGrid}
   DebugLnExit('TCustomGrid.SelectEditor END');
   {$endif}
grids.popup.diff (382 bytes)

Bart Broersma

2014-12-17 11:47

developer   ~0079863

Possible patch attached (grids.popup.diff).
@Jesus: please review.

procedure TCustomGrid.SelectEditor;
var
  aEditor: TWinControl;
begin
  {$ifdef DbgGrid}
  DebugLnEnter('TCustomGrid.SelectEditor INIT');
  {$endif}
  aEditor := GetDefaultEditor(Col);
  if EditingAllowed(FCol) and Assigned(OnSelectEditor) then begin
    // in some situations there are only non-selectable cells
    // if goAlwaysShowEditor is on set initially editor to nil,
    // user can modify this value in OnSelectEditor if needed
    if not SelectCell(FCol,FRow) then
      aEditor:=nil;
    OnSelectEditor(Self, fCol, FRow, aEditor);
  end;
  if aEditor<>Editor then
    Editor := aEditor;

//added this line
  if Assigned(Editor) then Editor.PopupMenu := PopupMenu;
//
  {$ifdef DbgGrid}
  DebugLnExit('TCustomGrid.SelectEditor END');
  {$endif}
end;

Cyrax

2014-12-17 16:54

reporter   ~0079868

What if editor does have its own PopupMenu created? With your current patch, it will be overridden by TCustomGrids own PopupMenu.

Bart Broersma

2014-12-17 17:59

developer   ~0079869

> What if editor does have its own PopupMenu created?
How/when does/can this happen?

Maybe?

if Assigned(Editor) and not Assigned(Editor.PopupMenu) then
  Editor.PopupMenu := PopupMenu;

Cyrax

2014-12-17 18:38

reporter   ~0079870

>How/when does/can this happen?
Anytime whenever developer wants that kind of functionality.

>if Assigned(Editor) and not Assigned(Editor.PopupMenu) then
> Editor.PopupMenu := PopupMenu;
Yes, that looks good.

Jesus Reyes

2014-12-17 18:46

developer   ~0079871

Yes, was my concern too. Please apply it.

Bart Broersma

2014-12-17 19:02

developer   ~0079872

Last edited: 2014-12-17 19:40

View 2 revisions

Maybe we can let the grid have an additional dedicated EditorPopupMenu property?

This way you can have popupmenu from the grid (set EditorPopUpMenu = PopUpMenu) and either a dedicated popupmenu for the editor, or the default OS popupmenu.
(With the above soulution it is always either grid popupmenu or OS default)

The logic would then be:

if Assigned(Editor) and not Assigned(Editor.PopupMenu) then
  Editor.PopupMenu := EditorPopupMenu;

See grids.popup.2.diff

Bart Broersma

2014-12-17 19:40

developer  

grids.popup.2.diff (4,166 bytes)
Index: lcl/grids.pas
===================================================================
--- lcl/grids.pas	(revision 47075)
+++ lcl/grids.pas	(working copy)
@@ -34,7 +34,7 @@
 uses
   Types, Classes, SysUtils, Math, Maps, LCLStrConsts, LCLProc, LCLType, LCLIntf,
   FileUtil, FPCanvas, Controls, GraphType, Graphics, Forms, DynamicArray,
-  LMessages, StdCtrls, LResources, MaskEdit, Buttons, Clipbrd, Themes,
+  LMessages, StdCtrls, LResources, MaskEdit, Buttons, Clipbrd, Themes, Menus,
   LazUtf8Classes, Laz2_XMLCfg; // <-- replaces XMLConf (part of FPC libs)
 
 const
@@ -675,6 +675,7 @@
     FEditorShowing: Boolean;
     FEditorKey: Boolean;
     FEditorOptions: Integer;
+    FEditorPopupMenu: TPopupMenu;
     FExtendedSelect: boolean;
     FFastEditing: boolean;
     FAltColorStartNormal: boolean;
@@ -796,6 +797,7 @@
     function  GetColWidths(Acol: Integer): Integer;
     function  GetColumns: TGridColumns;
     function  GetEditorBorderStyle: TBorderStyle;
+    function  GetEditorPopupMenu: TPopupMenu;
     function  GetBorderWidth: Integer;
     function  GetRowCount: Integer;
     function  GetRowHeights(Arow: Integer): Integer;
@@ -834,6 +836,7 @@
     procedure SetDefRowHeight(AValue: Integer);
     procedure SetDefaultDrawing(const AValue: Boolean);
     procedure SetEditor(AValue: TWinControl);
+    procedure SetEditorPopupMenu(Value: TPopupMenu);
     procedure SetFixedRows(const AValue: Integer);
     procedure SetFocusColor(const AValue: TColor);
     procedure SetGridLineColor(const AValue: TColor);
@@ -1094,6 +1097,7 @@
     property EditorMode: Boolean read FEditorMode write EditorSetMode;
     property EditorKey: boolean read FEditorKey write FEditorKey;
     property EditorOptions: Integer read FEditorOptions write SetEditorOptions;
+    property EditorPopupMenu: TPopupMenu read GetEditorPopupMenu write SetEditorPopupMenu;
     property EditorShowing: boolean read FEditorShowing write FEditorShowing;
     property ExtendedColSizing: boolean read FExtendedColSizing write FExtendedColSizing;
     property ExtendedRowSizing: boolean read FExtendedRowSizing write FExtendedRowSizing;
@@ -1288,6 +1292,7 @@
     property Editor;
     property EditorBorderStyle;
     property EditorMode;
+    property EditorPopupMenu;
     property ExtendedColSizing;
     property AltColorStartNormal;
     property FastEditing;
@@ -1425,6 +1430,7 @@
     property DragCursor;
     property DragKind;
     property DragMode;
+    property EditorPopupMenu;
     property Enabled;
     property ExtendedSelect;
     property FixedColor;
@@ -1605,6 +1611,7 @@
       property Cols[index: Integer]: TStrings read GetCols write SetCols;
       property DefaultTextStyle;
       property EditorMode;
+      property EditorPopupMenu;
       property ExtendedSelect;
       property Objects[ACol, ARow: Integer]: TObject read GetObjects write SetObjects;
       property Rows[index: Integer]: TStrings read GetRows write SetRows;
@@ -1642,6 +1649,7 @@
     property DragCursor;
     property DragKind;
     property DragMode;
+    property EditorPopupMenu;
     property Enabled;
     property ExtendedSelect;
     property FixedColor;
@@ -2429,6 +2437,13 @@
   {$endif}
 end;
 
+procedure TCustomGrid.SetEditorPopupMenu(Value: TPopupMenu);
+begin
+  FEditorPopupMenu := Value;
+  if FEditorPopupMenu <> nil then
+    FEditorPopupMenu.FreeNotification(Self);
+end;
+
 procedure TCustomGrid.SetFixedCols(const AValue: Integer);
 begin
   if FFixedCols=AValue then begin
@@ -4977,6 +4992,11 @@
     doTopleftChange(False)
 end;
 
+function TCustomGrid.GetEditorPopupMenu: TPopupMenu;
+begin
+  Result := FEditorPopupMenu;
+end;
+
 function TCustomGrid.IsCellButtonColumn(ACell: TPoint): boolean;
 var
   Column: TGridColumn;
@@ -5190,6 +5210,7 @@
   end;
 end;
 
+
 procedure TCustomGrid.SetTitleFont(const AValue: TFont);
 begin
   FTitleFont.Assign(AValue);
@@ -7967,6 +7988,8 @@
   end;
   if aEditor<>Editor then
     Editor := aEditor;
+  if Assigned(Editor) and not Assigned(Editor.PopupMenu) then
+    Editor.PopupMenu := EditorPopupMenu;
   {$ifdef DbgGrid}
   DebugLnExit('TCustomGrid.SelectEditor END');
   {$endif}
grids.popup.2.diff (4,166 bytes)

Jesus Reyes

2014-12-17 20:12

developer   ~0079874

I think is not necessary because most of the times what we want is that the editor share the grid's popup menu.
 
If an editor should have an special context menu (or not use one from the grid, or if a user editor is already configured with a particular popup menu) I think is a task for the OnSelectEditor event.

Bart Broersma

2014-12-17 23:10

developer   ~0079875

Yes, but in that scenario it is impossible to have a popupmenu for the grid _and_ to have the default OS popupmenu for the editor (which, for me, in most cases would suffice).
Also I would think that a grids popupmenu would most likely differ from a popupmenu for the editor (which I would expect to have at least copy/cut/paste) and the entries for the editor's popupmenu might not be applicable to the grid (in toto).

Jesus Reyes

2014-12-18 03:56

developer   ~0079882

I don't see then why a EditorPopUpMenu property should be added to the grid, if the user attach a custom popup menu to that property, it will override the "OS default popupmenu", which BTW, doesn't exists for all OS's (systems) that Lazarus targets. The current situation where the editor do not have a context menu is even ideal if the system should supply a context menu because it doesn't get overriden by user customization.

On the other hand, having this option how it would solve the problem on setting up a custom context menu and at the same time allowing the OS context menu?, AFAIK, it would have to be one or the other.

But it all really depends on what the user want to do, and the grid is component very flexible. In one use case, if the user want the editor to share the same grid's popup menu, then it have to assign it both to the grid popupmenu property and also in EditorPopupMenu property. in other use case, If the user want a per cell context menu, he should use OnSelect Editor, he would have to EITHER setup different editors per cell everyone already configured with a different context menu OR use the OnSelectEditor to setup different popups menus for every editor depending on the current cell. In either case the property EditorPopUpMenu serves nothing or it would have to be changed in the OnSelectEditor.

So I think the EditorPopupMenu could help in some cases but do nothing in other cases, just like the current situation.

My opinion is that we better document how to attach or detach a popup menu for every cell (including the current editor).

Bart Broersma

2014-12-18 20:50

developer   ~0079891

Last edited: 2014-12-19 09:14

View 2 revisions

> On the other hand, having this option how it would solve the problem on
> setting up a custom context menu and at the same time allowing the OS
> context menu?, AFAIK, it would have to be one or the other.

You misunderstood me.
I did not mean that in one program all these three possibilities should be available/apply, just that when developing (so in the designer) it would be nice to be able to choose between these scenario's: grid-popup = editor's popup vs grid-popup is one, editor's popup is something else (which may be nil).
All this without having to write code at designtime.
All this just to make life a little easier for the common people (as in: Lazarus users).
So, IMO it's about convenience.

Also, the first solution deprives people of having no popupmenu (or the OS default) one when the grid has a popupmenu. Or at least I don't see how to achieve this.

Yes, all this can be accomplished in OnSelectEditor, which would be the proper choice anyway if one wants to have different popups depending on e.g. the type of editor or the content/meaning of a certain cell.
But by that same reasoning, the first proposed "fix" is also unneccesary, since the programmer can achieve the same result in OnSelecteditor anyway.

Mind you, I can live with both solutions, so I ultimately leave the decision up to you (you know grids better than me).

Bart Broersma

2014-12-28 11:59

developer   ~0079992

Implemented as described in note 0079872.
Please test and close if OK.

Bart Broersma

2014-12-28 13:09

developer   ~0079994

Silly me, it was my own report.

Issue History

Date Modified Username Field Change
2014-12-16 20:47 Bart Broersma New Issue
2014-12-16 20:48 Bart Broersma File Added: sg.zip
2014-12-17 03:42 Jesus Reyes Assigned To => Jesus Reyes
2014-12-17 03:42 Jesus Reyes Status new => assigned
2014-12-17 11:46 Bart Broersma File Added: grids.popup.diff
2014-12-17 11:47 Bart Broersma Note Added: 0079863
2014-12-17 16:54 Cyrax Note Added: 0079868
2014-12-17 17:59 Bart Broersma Note Added: 0079869
2014-12-17 18:38 Cyrax Note Added: 0079870
2014-12-17 18:46 Jesus Reyes Note Added: 0079871
2014-12-17 18:46 Jesus Reyes Assigned To Jesus Reyes => Bart Broersma
2014-12-17 19:02 Bart Broersma Note Added: 0079872
2014-12-17 19:40 Bart Broersma File Added: grids.popup.2.diff
2014-12-17 19:40 Bart Broersma Note Edited: 0079872 View Revisions
2014-12-17 20:12 Jesus Reyes Note Added: 0079874
2014-12-17 23:10 Bart Broersma Note Added: 0079875
2014-12-18 03:56 Jesus Reyes Note Added: 0079882
2014-12-18 20:50 Bart Broersma Note Added: 0079891
2014-12-19 09:14 Bart Broersma Note Edited: 0079891 View Revisions
2014-12-28 11:59 Bart Broersma Fixed in Revision => r47258
2014-12-28 11:59 Bart Broersma LazTarget - => 1.2.8
2014-12-28 11:59 Bart Broersma Note Added: 0079992
2014-12-28 11:59 Bart Broersma Status assigned => resolved
2014-12-28 11:59 Bart Broersma Fixed in Version => 1.2.8
2014-12-28 11:59 Bart Broersma Resolution open => fixed
2014-12-28 11:59 Bart Broersma Target Version => 1.2.8
2014-12-28 13:09 Bart Broersma Note Added: 0079994
2014-12-28 13:09 Bart Broersma Status resolved => closed