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.
| Summary: | "Convert to composite component" is greyed out | ||
|---|---|---|---|
| Product: | javaee | Reporter: | edburns <edburns> |
| Component: | JSF | Assignee: | Marek Fukala <mfukala> |
| Status: | VERIFIED FIXED | ||
| Severity: | normal | CC: | dkonecny, jsedek, mfukala, mmocnak, mschovanek, pjiricka, sustaining |
| Priority: | P3 | ||
| Version: | 6.x | ||
| Hardware: | All | ||
| OS: | All | ||
| Issue Type: | DEFECT | Exception Reporter: | |
There are basic check of the selection implemented, so it will be grayed out if your selected text doesn't contain open and close tags. Mark as incomplete. Alexey, why this was closed as incomplete? If I take given code and paste it into index.xhtml and select it then 'Convert to Composite Component' is disabled despite selection starting before opening tag and ending after closing tag. So the problem is reproducible and your explanation is not applicable for this case. I debugged this quickly and there seems to be at least two problems:
#1) if xhtml contains expression language then htmlResult.elementsList() returns tokens with offsets which do not correspond to real document offset. Marek knows more whether these offset needs to be translated to real ones or whether something else then htmlResult.elementsList() should be called instead or whether there is a bug somewhere else. To test this just use code snippet provided by Ed and dump tokens and you will see for example that instead of real tokens:
Element(tag)[591,647] "<h:inputSecret value="#{model.password}">" - "h:inputSecret" - {TagAttribute[name=value; value="#{model.password}"; nameOffset=622; valueOffset=628], }
Element(endtag)[648,663] "</h:inputSecret>" - "h:inputSecret" - {}
you will get
Element(tag)[591,633] "<h:inputSecret value="@@@">" - "h:inputSecret" - {TagAttribute[name=value; value="@@@"; nameOffset=622; valueOffset=628], }
Element(endtag)[634,649] "</h:inputSecret>" - "h:inputSecret" - {}
Consequently because of shifted offsets wrong tags are considered being "selected" and action is disabled.
#2) logic which enables/disables action does not take into account tags in format <tag/> - it always expect separate opening and closing tag
There is no workaround I'm aware of part from not using EL (in whole document) and always open and close tags.
Clearly this feature was not tested at all before release and would be good to include it in some test plans/automated tests.
Davide do we have a spec or description for this feature? After some discussion with the QE we agreed on following behaviour: 1) the Convert to Composite component popup menu will be removed 2) Selection sensitive code hint "Convert to Composite components" will be added 3) New insert code action "Inject Composite Component" will be added The same template dialog will be opened for both action, in the first case there will be the selected code shown in the bottom textarea 4) for #1 there will be a warning in the dialog notifying the user that the selection contains not paired *JSF* tags (html tags matching doesn't matter), with a notice that the generated code may be invalid. However the wizard will allow to finish. Following bugs also needs to be fixed: 1) hardcoded "ez" prefix - new input field will be added to the visual panel. Existing prefixes needs to be checked to avoid clashes. 2) missing second step name in the wizard 3) missing code coloring in the "Implementation section" textarea in the wizard 4) use of JSF "auto tag import" instead of fifty lines of proprietary code to insert the desired namespace declaration web-main#ab8ff28ea9b3 proposed functionality changes #1-4 has been done. bug #4 fixed. and of course the algorithm to find if the selection is valid or not has been reimplemented and is now embedding aware. What matters now is that there aren't any partially selected JSF tags, html content doesn't play any role. I'm keeping the issue opened as P3 since bugs #1-3 are still valid. Integrated into 'main-golden', will be available in build *200912190200* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress) Changeset: http://hg.netbeans.org/main/rev/ab8ff28ea9b3 User: Marek Fukala <mfukala@netbeans.org> Log: #178751 (partial fix) - "Convert to composite component" is greyed out re. "Davide do we have a spec or description for this feature?" - if there is one then Marek or Alexey know about it. re. "replacing Convert to Composite component action with editor hint" - I have not tested the new implementation and I do not know arguments based on which you've decided to implement it like this but I tend to disagree. Convert to Composite Component (C2CC) is not *hint*. Hint is an advice editor can give you to improve your code or alert you about commonly made mistakes etc. Just selecting some code should never give you hint "convert this to composite components" - that's not advice. In that sense I think that C2CC is action which user decides to perform similarly as most of the refactoring actions for example. Also something what initially was one single action - turn this selected code snippet into CC and replace it with CC call - is exactly what user wants to do and in that sense we are breaking it into two separate actions is step in wrong way instead of improvement of existing feature. Perhaps I'm just missing details of your discussion based on which you've decided for these changes but so far it looks like implementation difficulties/issues are driving IDE's UI and what user actually wants is being ignored. re. "3) New insert code action Inject Composite Component" - 'inject' is sexy word which everybody seems to like to use these days, but I do not think it is accurate in this context. All what action does is that it *inserts* call of CC, right? If that's true then this Insert Code item should be called "Composite Component Call" or "Call Composite Component" or ... Could we also keep this as P2 and file other problems as separate P3 issue(s)? I would like to make sure it does not slip from Sustaining radar and gets fixed in a patch. It is not essential feature without users could not use IDE but composite components is new JSF 2.0 feature which NetBeans should propagate or advertise and right now it is completely broken and unusable. Sorry if this response sounds overly negative. It is not how it is meant. Marek, QE, Alexey, ... you are doing great job addressing editor issues so promptly and me, IDE users, ... we are appreciating it! :-) Thanks. (In reply to comment #8) > re. "Davide do we have a spec or description for this feature?" - if there is > one then Marek or Alexey know about it. I am not aware of any, maybe Alexey has one. > > re. "replacing Convert to Composite component action with editor hint" - I have I tend to agree with you David, at least from the point that the C2CC is kind of refactoring an as such should be consistent with the rest of the editors. OTOH the action "Convert to Composite Component" itself being in the popup looks at least weird if not stupid IMO. Sure we can introduce Refactoring -> C2CC action so it at least looks similar as the rest of refactorable editors. But my impression from this arrangement is that it is too flagrant that that we simply have no refactoring, which makes no good impression :-). So the hint was a kind of workaround for that. Once we do the html/xhtml/jsf refactorings (hopefully in 6.9) there will be "Refactoring" popup where the C2CC will logically be. > > re. "3) New insert code action Inject Composite Component" - 'inject' is sexy > word which everybody seems to like to use these days, but I do not think it is > accurate in this context. All what action does is that it *inserts* call of CC, > right? Not exactly, it creates the composite component and adds a reference to it which is what most people considers as "inject" AFAIK. However if you look at the real implementation or source you'll realize there is just "Composite Component" in the Generate "insert code menu" which seems to be fine to me. MSG_InjectCompositeComponentHint=Composite Component Call to Composite Component is really ugly anyway :-) > Could we also keep this as P2 and file other problems as separate P3 issue(s)? > I would like to make sure it does not slip from Sustaining radar and gets fixed Yes, so then I consider this as fixed. > in a patch. It is not essential feature without users could not use IDE but > composite components is new JSF 2.0 feature which NetBeans should propagate or > advertise and right now it is completely broken and unusable. > > Sorry if this response sounds overly negative. It is not how it is meant. > Marek, QE, Alexey, ... you are doing great job addressing editor issues so > promptly and me, IDE users, ... we are appreciating it! :-) Thanks. Not offended at all, I like your ideas. Thanks! MF> After some discussion with the QE we agreed on following behaviour: MF> 1) the Convert to Composite component popup menu will be removed MF> 2) Selection sensitive code hint "Convert to Composite components" will be MF> added MF> 3) New insert code action "Inject Composite Component" will be added MF> OTOH the action "Convert to Composite Component" itself being in the MF> popup looks at least weird if not stupid IMO. [discussion between David and Marek deleted] I suggest that the name be chosen to make it clear that some text in the page will be replaced with the composite component. The name "Inject Composite Component" does not say that to me, but "Convert to Composite Component" does. I agree it is a refactoring and that it was weird to have it in the popup. The "advice" option is decent, but David had some reasons why it was not quite right. Perhaps the refactoring menu is the right place. MF> 1) hardcoded "ez" prefix - new input field will be added to the visual panel. MF> Existing prefixes needs to be checked to avoid clashes. MF> 2) missing second step name in the wizard MF> 3) missing code coloring in the "Implementation section" textarea in the wizard MF> 4) use of JSF "auto tag import" instead of fifty lines of proprietary code to MF> insert the desired namespace declaration These are all good changes. One more is to use "cc" as the prefix in the composite component page, rather than "composite". Thanks. I'm playing with latest dev build and here are some more comments: "Selection sensitive code hint 'Convert to Composite components'" - the name of the hint in today's build is just "Composite components". It works well but I worry about discoverability of this feature. Hint icon in the editor gutter is the same icon for all hints which does not help either. The name is pretty vague too. I would rather have Refactoring menu with one action (named "Convert to Composite Component") than hiding this action from user. Some people might think it looks empty to have just one refactoring action but I would argue that it is better than no action. :-) "[Inject] Composite Component" - naming is OK. After CC is created and inserted I would expect to get CC source opened in editor and be ready for editing. Do you think Marek that your fix (I have not looked at it) is simple enough to be backported to 68 patch? (In reply to comment #11) > Thanks. I'm playing with latest dev build and here are some more comments: > > "Selection sensitive code hint 'Convert to Composite components'" - the name of > the hint in today's build is just "Composite components". Yes, this was a "typo". I've changed the hint's description to the same text as its fix has. > worry about discoverability of this feature. Hint icon in the editor gutter is > the same icon for all hints which does not help either. The name is pretty > vague too. Is it still so vague when changed to "Convert to Composite Component"? > I would rather have Refactoring menu with one action (named "Convert > to Composite Component") than hiding this action from user. Some people might > think it looks empty to have just one refactoring action but I would argue that > it is better than no action. :-) OK. I'll add Refactor -> Convert to Composite Component items. > > "[Inject] Composite Component" - naming is OK. After CC is created and inserted > I would expect to get CC source opened in editor and be ready for editing. Yes, that might be better. I'll address it along with the other changes (bugs #1-3). > > Do you think Marek that your fix (I have not looked at it) is simple enough to > be backported to 68 patch? I guess for 6.8 it would be better not to change the UI and just backport the fix of the algorith deciding whether the action should be enabled or disabled. What do you think? In such case, I'll do the backport myself. The issue is already marked as patch candidate. > MF> OTOH the action "Convert to Composite Component" itself being in the > MF> popup looks at least weird if not stupid IMO. > > [discussion between David and Marek deleted] > > I suggest that the name be chosen to make it clear that some text in the > page will be replaced with the composite component. The name "Inject > Composite Component" does not say that to me, but "Convert to Composite > Component" does. There isn't and has never been the word "Inject" used in the jsf sources. I used the term in the issue description, but then realized that the "insert code menu has already a title "Generate" so used just "Composite Component" description. So the discussion makes no sense. I'm sorry for the confusion. > These are all good changes. One more is to use "cc" as the prefix in the > composite component page, rather than "composite". I've taken the original library prefixes from the TLDs AFAIR, but I can change it of course. I've fixed the wording, the default composite library prefix and added the refactoring action + the editor submenu. However I cannot push the change because of the Internal Server Error. changeset: 156088:56ab6f0abb85 user: Marek Fukala <mfukala@netbeans.org> date: Wed Dec 23 12:48:00 2009 +0100 summary: C2CC hint has now the same description as its fix - conver to composite component changeset: 156230:b037303ce61f user: Marek Fukala <mfukala@netbeans.org> date: Mon Jan 04 13:49:27 2010 +0100 summary: using "cc" as default composite components library prefix changeset: 156232:151b98f3abea tag: tip user: Marek Fukala <mfukala@netbeans.org> date: Mon Jan 04 17:58:30 2010 +0100 summary: #178751 - adding Refactoring action "Convert To Composite Component" for JSF xhtml files, also adding the Refactor submenu into editor popup changeset: 156233:8ef15fbd7dd7 tag: tip user: Marek Fukala <mfukala@netbeans.org> date: Wed Jan 06 12:59:26 2010 +0100 summary: #178751 - fixing wizard step name, fixing issue in the panel layout, replacing the plain preview by html editor's editor pane. changes finally pushed to web-main. AFAIK the only undersolved issue now remains: 1) hardcoded "ez" prefix - new input field will be added to the visual panel. Existing prefixes needs to be checked to avoid clashes. Which I belive it P3.5 issue. Can this one (P2) be closed then? Aha, I've just realized the issue is P3 only, closing then. Reopen if you have any objections to the solution. Thanks all for the feedback! I'll try this now. I downloaded web-main build 2331 yesterday, but I guess this fix was not in it. Integrated into 'main-golden', will be available in build *201001070200* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress) Changeset: http://hg.netbeans.org/main/rev/151b98f3abea User: Marek Fukala <mfukala@netbeans.org> Log: #178751 - adding Refactoring action "Convert To Composite Component" for JSF xhtml files, also adding the Refactor submenu into editor popup I tried to verify, but I faced an issue #179377 that should be fixed and integrated into patch with this issue as well I've finally fixed the last remaining problem with hardcoded prefix. There's a new input field where user can specify the library prefix, default valud is generated from the library's folder name. Checks for already used prefixes added. fixed in web-main#03ac3a99c553 also note fix of issue #179379 - (convert to composite component doesn't generate correct JSF namespaces), which is a kind of "enhancement" :-). A side effect of this fix is that unused namespaces are now not created in the created composite component's code. verified. NetBeans IDE Dev (Build 201001180201) Hi, I used to go to http://bertram.netbeans.org/hudson/job/web-main/ to get my latest builds but that doesn't seem to be up any more. Where should I go now? There are ongoing problems with the Bertram machine, you can use http://bits.netbeans.org/netbeans/trunk/nightly/ instead. The fix has been ported into the release68_fixes repository. http://hg.netbeans.org/release68_fixes/rev/f23c127544e3 The fix has been ported together with the bugfix of issue #179379. If a rollback is needed, both issues have to be skipped together. verified in 68_fixes build NetBeans IDE 6.8 (Build 201001261800) Ok, I see this working now. Thank you very much. |
I created a simple JSF Project with facelets and added the following code into my index.xhtml page: <h:panelGrid columns="2"> <h:outputText value="Userid:" /> <h:inputText value="#{model.userid}" /> <h:outputText value="Password:"/> <h:inputSecret value="#{model.password}" /> <h:commandButton binding="#{model.actionSource}" value="Login" /> </h:panelGrid> I highlighted the text and was about to click, "Convert to composite component" but I saw that it was greyed out! Oof, this is my favorite feature! Can someone fix it fast?