This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 95110 - Visual Library API review
Summary: Visual Library API review
Status: RESOLVED FIXED
Alias: None
Product: platform
Classification: Unclassified
Component: Graph (show other bugs)
Version: 3.x
Hardware: All All
: P1 blocker with 1 vote (vote)
Assignee: apireviews
URL:
Keywords: API_REVIEW
Depends on: 97562 97563 97564
Blocks: 90459 93848 95473
  Show dependency tree
 
Reported: 2007-02-09 16:16 UTC by David Kaspar
Modified: 2007-03-15 14:53 UTC (History)
5 users (show)

See Also:
Issue Type: TASK
Exception Reporter:


Attachments
Notes from MDŽ review meeting on 8. 3. 2007 (2.86 KB, text/plain)
2007-03-08 17:50 UTC, Jaroslav Tulach
Details

Note You need to log in before you can comment on or make changes to this bug.
Description David Kaspar 2007-02-09 16:16:06 UTC
This is a request for review of Visual Library 2.0 API.

The sources and javadoc is available in the main trunk of the "graph/lib" module
in NetBeans CVS.

API documentation:
http://graph.netbeans.org/documentation.html

Project home page:
http://graph.netbeans.org/
Comment 1 Nathan Fiedler 2007-02-20 20:44:15 UTC
I hope it is not too late to consider renaming a couple of the layout classes. 
  It seems like SerialLayout should be called FlowLayout, so developers can
easily relate it to the Swing layout and quickly understand what it does.
Likewise, I feel FillLayout should be called OverlayLayout, as "fill" does not
seem to fit the description of what it does.
Comment 2 David Kaspar 2007-02-20 22:45:25 UTC
I think that many people are using current names and I would like to not harm
their code unless it is necessary. Anyway all names could be changed in the
documentation. In the public API, we can introduce additional
"LayoutFactory.create*Layout" methods and optionally deprecate the old factory
methods.

SerialLayout:
could be renamed to FlowLayout in documentation. In the API there are
"LayoutFactory.createHorizontalLayout" and "LayoutFactory.createVerticalLayout".
 Maybe we can introduce additional "createHorizontalFlowLayout" and
"createVerticalFlowLayout" methods too.

FillLayout:
could be renamed to OverlayLayout in documentation, a new
"LayoutFactory.createOverlayLayout" method may be added.
Comment 3 maxnitribitt 2007-02-21 16:01:39 UTC
The documentation states that that dependencies are meant for listening "on a
widget move or resize changes". In that case I would expect, that
revalidateDependency is called after the layout is completed and resolveBounds
has been called, so I can get the updated bounds and location when calling
getBounds, but during the validation process the revalidatDependency is called
before the layout takes effect:
4. All widget dependencies are notified about revalidation.
5. Each modified widget evaluates its location and boundary using its layout. 

The documentation for Widget Dependency is correct as it links to the validation
section, but an additional hint would make it easier to comprehend.
Comment 4 David Kaspar 2007-02-22 16:17:06 UTC
I have slightly changed the WidgetDependency section in the documentation. I do
not know whether it is described enough. Please, let me know, if not. Thanks.
Comment 5 maxnitribitt 2007-02-23 10:29:52 UTC
Most of the Widget methods are final, which is sometimes inconvenient. 

For example, when a user wants to do something (e.g. prevent a certain widget
from being disabled, or enable/disable children) when setEnabled is called. Or
for  setVisible: A user e.g. might want to load resources for the widget, only
when it is set visible and discard them when it is set invisible. It would be
more convenient if these (and possibly other) methods could be overridden.

Comment 6 Joelle Lam 2007-02-27 22:54:55 UTC
In Graph Library's current implementation, Widgets use WidgetActions to define
their behavior.  However, developers of JComponents often use and reuse
ActionMaps/InputMaps etc. Current Widget implementation does not have built-in
support for ActionMaps.

Should widget have built-in support for ActionMap?

An Example Use Case:
When a Widget is selected, if the user selects 'B', it adds a button widget to
the widget.  This same action is reused and called from a JPopup on that widget.
 Right now there is no way to reuse this action.  There are workarounds, ie
creating my own Widget type that implements an actionMap, etc.  But I think this
may be a common use case.  Does anyone else have any suggestions on this topic?
Comment 7 David Kaspar 2007-03-02 14:10:49 UTC
This "final" keyword is used on purpose. Actually if there is a "public"
"non-static" method without "final" modifier, then it is a bug.

If those methods would be "non-final", then the library cannot rely on its
behaviour (e.g. overring a setter may disallow setting the value, ...). This may
lead to unexpected behaviour and makes it hard to trace bugs in the library.

Also there could be another method introduced in the future with a similar
behavior as the original one has.
E.g. There is "Widget.addChild(Widget)" method from beginning. Later there was
"addChild(Widget,Object)" method added. If you would be able override the
original "Widget.addChild(Widget)", then after the new method introduction, you
(as an user of the library) would be forced to override the new method too.

Rather I would recomend you to create your own method that calls the original
method. This means you would have to introduce a new method and make sure that
you are calling your own method.

The current pattern has an advantage that it allows future extensions. E.g. if
you decide and a new "hook/notify" method like "notifyAdded" has to be
introduced, then it could be added later.
Comment 8 David Kaspar 2007-03-03 11:23:15 UTC
Answer for ActionMap question:
http://graph.netbeans.org/servlets/ReadMsg?list=users&msgNo=538
Comment 9 Joelle Lam 2007-03-05 20:02:44 UTC
Response for ActionMap questions
http://graph.netbeans.org/servlets/ReadMsg?listName=users&msgNo=538
Comment 10 Jaroslav Tulach 2007-03-07 11:34:50 UTC
Y01 publish javadoc - right now I do not know where to find javadoc, I had to 
build it myself - publish it on our regular website, how to linked from there. 
If you do it today, it can be uploaded tommorrow morning.

Y02 crosslink documentation.html and javadoc - when there is a name of class, 
use <a href> to it and possibly also link from classes javadoc back to 
documentation.html - imho this is best achieved if documentation.html is 
included directly in the javadoc ZIP file.

Y03 stability of the API - I assume you want "stable" - so export it using 
<api/> tag in the arch.xml, so it is visible in the javadoc.

Y04 write apichanges.xml and put there at least one change - e.g. since 
day/month/year this API is stable - this way the API get's listed in the page 
of changes since previous release

Y05 target cluster for 6.0? What cluster you want this be part of? platform?

Y06 There is a lot of public packages (that I nearly got scared), please 
provide package.html description for each of them.

Y07 make sure the signature testing is running on your library - this probably 
means to talk to RE, especially Robert Novak, on setting up the check

Y08 show automated testing - before this gets into platform and potentially 
other people than you share responsibility for the code - I want to make sure 
it can be automatically tested. You do not need to have 90% coverage, but 
write sample tests showing how to test various constaints embedded into the 
code.

Otherwise ok, as the whole library is too big to really comment on it without 
trying it...
Comment 11 Jaroslav Tulach 2007-03-07 11:36:27 UTC
Y09 if you feel that there are classes that are not final yet - like 
LookFeel - either make them package private of hide their package from the 
list of public packages - or declare them stable.
Comment 12 Jaroslav Tulach 2007-03-07 13:45:29 UTC
Y10 I have noticed a lot of classes VMD* - what is VMD? Does this belong into 
API? If so, then please add some explanation to the VMD classes javadoc.
Comment 13 David Kaspar 2007-03-08 14:57:29 UTC
Y01 - in progress - it should be visible from tomorrow

Y02 - agree, will be done ASAP

Y03 - done

Y04 - done

Y05 - I propose to include it into "platform" cluster for 6.0 release.

Y06 - agree, will be done ASAP

Y07 - in progress

Y08 - Automated visual test has been added into the graph/lib module. It covers
just basic scene rendering. Rather take it as an example of automated-visual-test.

Y09 - I have updated javadoc of particular classes. Now there are only
ListWidget and ListItemWidget classes marked as deprecated. I can move it out
from the public API and put them into the examples or experimental module
instead. Note that this would be an incompatible-API-change between the
release551 branch and the main trunk.

Y10 - VMD is an acronym for Visual Mobile Designer. In the library is used as a
shortcut for a particular visualization and UI look&feel.
Comment 14 Jaroslav Tulach 2007-03-08 17:50:27 UTC
Created attachment 39314 [details]
Notes from MDŽ review meeting on 8. 3. 2007
Comment 15 David Kaspar 2007-03-09 13:16:41 UTC
Added dependency on TCR issues.
Comment 16 David Kaspar 2007-03-09 17:06:17 UTC
New status of Y* questions:
Y02 - will be done ASAP
Y06 - done
Y09 - done
Y10 - done

Also deprecated methods/classes has to be removed for NB 6.0 Milestone 9.
Comment 17 David Kaspar 2007-03-15 14:53:12 UTC
Y02: Documentation is a part of javadoc at
org/netbeans/api/visual/widget/doc-files/documentation.html.

Resolving as fixed.