View Issue Details
| ID | Project | Category | View Status | Date Submitted | Last Update |
|---|---|---|---|---|---|
| 0036956 | Lazarus CCR | Packages | public | 2020-04-22 22:12 | 2020-04-24 20:12 |
| Reporter | Michal Gawrycki | Assigned To | wp | ||
| Priority | normal | Severity | minor | Reproducibility | have not tried |
| Status | resolved | Resolution | fixed | ||
| Summary | 0036956: jvcl - TJvNotebookPageList - some corrections. | ||||
| Description | I 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. | ||||
| Tags | No tags attached. | ||||
| Widgetset | |||||
| Attached Files |
| ||||
|
|
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); |
|
|
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... |
|
|
> 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. |
|
|
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. |
| 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 |
