View Issue Details

IDProjectCategoryView StatusLast Update
0036956Lazarus CCRPackagespublic2020-04-24 20:12
ReporterMichal Gawrycki Assigned Towp  
PrioritynormalSeverityminorReproducibilityhave not tried
Status resolvedResolutionfixed 
Summary0036956: jvcl - TJvNotebookPageList - some corrections.
DescriptionI am sending a patch that changes:

1. TJvNotebookPageList.AddPage - Use only Pages.Add, which itself set name and caption of the new page.

2. TJvNotebookPageList.GetPageCaption, TJvNotebookPagelist.PageCaptionChanged - property Pages contains page names (component name), not captions. We need to get/set Caption from Page property.

3. TJvNotebookPageList.MovePage - Use the Move (0036953) method instead of Exchange.
TagsNo tags attached.
Widgetset
Attached Files

Activities

Michal Gawrycki

2020-04-22 22:12

reporter  

jvnotebookpagelist.patch (1,307 bytes)   
Index: run/JvPageComps/jvnotebookpagelist.pas
===================================================================
--- run/JvPageComps/jvnotebookpagelist.pas	(revision 7396)
+++ run/JvPageComps/jvnotebookpagelist.pas	(working copy)
@@ -72,13 +72,8 @@
 -------------------------------------------------------------------------------}
 
 procedure TJvNotebookPageList.AddPage(const ACaption: String);
-var
-  idx: Integer;
-  lPage: TPage;
 begin
-  idx := Pages.Add(ACaption);
-  lPage := Page[idx];
-  lPage.Name := GetUniqueName(Self, 'TPage');
+  Pages.Add(ACaption);
 end;
 
 function TJvNotebookPageList.CanChange(AIndex: Integer): Boolean;
@@ -95,7 +90,7 @@
 
 function TJvNotebookPageList.GetPageCaption(AIndex: Integer): string;
 begin
-  Result := Pages[AIndex];
+  Result := Page[AIndex].Caption;
 end;
 
 function TJvNotebookPageList.GetPageCount: Integer;
@@ -105,13 +100,13 @@
 
 procedure TJvNotebookPageList.MovePage(CurIndex, NewIndex: Integer);
 begin
-  Pages.Exchange(CurIndex, NewIndex);
+  Pages.Move(CurIndex, NewIndex);
 end;
 
 procedure TJvNotebookPagelist.PageCaptionChanged(AIndex: Integer;
   const NewCaption: string);
 begin
-  Pages[AIndex] := NewCaption;
+  Page[AIndex].Caption := NewCaption;
 end;
 
 procedure TJvNotebookPageList.SetActivePageIndex(AIndex: Integer);
jvnotebookpagelist.patch (1,307 bytes)   

wp

2020-04-23 11:36

developer   ~0122351

Last edited: 2020-04-23 11:38

View 3 revisions

I will probably reject your idea # 3 to replace TJvNotebookPageList.Exchange by .Move since it makes JVCL depend on a current Lazarus version. I try to avoid such things as possible, because there are so many users out there using old, sometimes ancient Lazarus versions (although I could not tell at the moment about the minimum Lazarus version for JVCL).

Do you have a particular application in mind which causes trouble? In this case, post a sample project.

I am not sure about TJvNotebookPageList at all. IIRC, I saw that there are many nice JVCL components (TJvTreeView, TJvTabBar) which depend on TJvPageList. Trying to port JvPageList I ran into lots of trouble with the property editors (not necessarily because of TJvPageList itself, but mostly because I did not understand our property editors very well - and it's not much better today). Then I noticed that TJvPageList must be very similar to our TNotebook. Trying to still use the JvPageList infrastructure, but replace JvPageList by TNotebook, I introduced TJvNotebookPageList. I am not sure if this has been done correctly.

So, I think putting much work into TJvNotebookPageList could be wasted time. I see two directions: Either we port TJvPageList correctly and get the property editor to work, or - with some loss in generality - we adapt the JvPageList-dependent components (JvPageListTreeView, JvTabBar) to work with TNotebook instead of TJvPageList.

Our TNotebook has an issue when a tab is added with a caption corresponding to the name of an existing component - I'll post a report about it...

Michal Gawrycki

2020-04-23 12:20

reporter   ~0122354

> Do you have a particular application in mind which causes trouble? In this case, post a sample project.
You can see JvTabBarDemo_NotebookPages.lpr example project. You can now move tab in this project, but order of notebook pages remain unchanged because property TJvTabBar.PageListTabLink is false. But if you set PageListTabLink propert to true, after moving tab, notebook page is destroyed instead of reordered. Therefore, we should use Move method instead of Exchange.

You're right that we should improve TJvPageList component, but until we do it, TJvNotebookPageList is the only working replacement.
I'll see what I can do with TJvPageList.

wp

2020-04-24 20:12

developer   ~0122391

Applied your patch. Regarding # 3 I was not aware of the trivial fact that your code compiles with the release version although it does not fix the page-exchange issue.

I tried for some time to find a way how to exchange the order of TUNBPages items without changing the notebook sources, but failed, even with streaming. TNotebook is not very flexibile, not a pretty good example of OOP... If you know a way how to make the component work with the current Laz release you should reopen the report. Otherwise test and close if ok.

Issue History

Date Modified Username Field Change
2020-04-22 22:12 Michal Gawrycki New Issue
2020-04-22 22:12 Michal Gawrycki File Added: jvnotebookpagelist.patch
2020-04-23 10:12 wp Assigned To => wp
2020-04-23 10:12 wp Status new => assigned
2020-04-23 11:36 wp Note Added: 0122351
2020-04-23 11:36 wp Status assigned => feedback
2020-04-23 11:37 wp Note Edited: 0122351 View Revisions
2020-04-23 11:38 wp Note Edited: 0122351 View Revisions
2020-04-23 12:20 Michal Gawrycki Note Added: 0122354
2020-04-23 12:20 Michal Gawrycki Status feedback => assigned
2020-04-24 20:12 wp Note Added: 0122391
2020-04-24 20:12 wp Status assigned => resolved
2020-04-24 20:12 wp Resolution open => fixed