Bug 139633 - Direct editing of tree nodes in the Navigator to rename objects
Summary: Direct editing of tree nodes in the Navigator to rename objects
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: UI (show other bugs)
Version:
(earliest affected)
7.2.0.0.alpha0+
Hardware: All All
: low enhancement
Assignee: Not Assigned
URL:
Whiteboard: target:7.5.0 target:24.2.0
Keywords: difficultyInteresting, easyHack, skillCpp, topicUI
Depends on: 156610
Blocks: Navigator
  Show dependency treegraph
 
Reported: 2021-01-15 10:49 UTC by Telesto
Modified: 2024-03-07 06:20 UTC (History)
7 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Telesto 2021-01-15 10:49:34 UTC
Description:
Rename in navigator opens a dialog? I'm used to directly input text in tree

Steps to Reproduce:
1. Open Writer
2. Insert an image/ shape/ drawing object/ frame
3. Sidebar -> Navigator -> Right click the frame/shape/image & select rename

Actual Results:
Dialog opens

Expected Results:
No dialog but directly editing inside the navigator

[Also like the use of F2 here the to enable rename]


Reproducible: Always


User Profile Reset: No



Additional Info:
Version: 7.2.0.0.alpha0+ (x64)
Build ID: f2171af6ce3516598d9f8bac8294025a21a5b1a2
CPU threads: 4; OS: Windows 6.3 Build 9600; UI render: default; VCL: win
Locale: nl-NL (nl_NL); UI: en-US
Calc: CL
Comment 1 V Stuart Foote 2021-01-15 15:40:34 UTC
It follows GUI convention, the label is "Rename..." indicating a dialog. I would assume it was a dev choice for reworked context menu for Sidebar objects to provide a dialog rather than direct edits.

Let's not get into second guessing the implementation choices devs make when there is no functional difference in usage.

WFM and => NAB
Comment 2 Telesto 2021-01-17 11:15:57 UTC
(In reply to V Stuart Foote from comment #1)
> It follows GUI convention, the label is "Rename..." indicating a dialog. I
> would assume it was a dev choice for reworked context menu for Sidebar
> objects to provide a dialog rather than direct edits.
> 
> Let's not get into second guessing the implementation choices devs make when
> there is no functional difference in usage.
> 
> WFM and => NAB

Lets say changing folder name or working with tree note taker doesn't function this way. This feels to me like GUI relic of the past..

 I'm always getting a kind of 1998 Linux/macOS feeling seeing a rename dialog (but not totally sure.. pretty sure Windows 95 didn't do it this way). Windows 3.1 might have rename dialog.. at least wouldn't be surprised :P

So obviously priority being low; functional. But this makes LibreOffice feel dated. Yes, application is old.. but keeping this around?

FWIW: not sure if the dialog being more 'logical' choice on LOOL/COOL side. 

I admit, from the technical side no issue (as comment 1). From aesthetical/perceptional/ user experience side.. I doubt this being desirable.. As said if have noticed this kind of behavior in long long time.

There should even be a generation who simply can't explain it :P
Comment 3 Heiko Tietze 2021-01-20 09:46:43 UTC
Direct editing the tree nodes is not uncommon (would expect F2 to switch into edit mode). 

Is it easy to do, Jim?
Comment 4 Jim Raykowski 2021-01-21 10:10:39 UTC
Direct editing is the way renaming is done in the Form Navigator. Menu > Form > Form Navigator... 

IMHO, it is a challenging hack for a beginning LO hacker.

If anyone is interested in taking this on please let me know. I will provide code  pointers and review to make this work in the Writer Navigator content tree.
Comment 5 Telesto 2021-01-21 12:48:56 UTC
Adding easyHack & mentoring.. To be sure it's on a proper list. Where 'easyHack' must be read 'instrumentally'; not literally.. being a challenging hack
Comment 6 Xisco Faulí 2021-01-21 20:40:47 UTC
(In reply to Telesto from comment #5)
> Adding easyHack & mentoring.. To be sure it's on a proper list. Where
> 'easyHack' must be read 'instrumentally'; not literally.. being a
> challenging hack

Hi Telesto,
Please, do not add the keyword 'easyhack' unless you know the code involved and the effort it would require to fix the issue. You also need to add code pointers when you use the keyword 'easyhack' and they are missing in this case. Deleting the keyword...
Comment 7 Telesto 2021-01-21 21:03:25 UTC
(In reply to Xisco Faulí from comment #6)
> (In reply to Telesto from comment #5)
> > Adding easyHack & mentoring.. To be sure it's on a proper list. Where
> > 'easyHack' must be read 'instrumentally'; not literally.. being a
> > challenging hack
> 
> Hi Telesto,
> Please, do not add the keyword 'easyhack' unless you know the code involved
> and the effort it would require to fix the issue. You also need to add code
> pointers when you use the keyword 'easyhack' and they are missing in this
> case. Deleting the keyword...

comment 4
Comment 8 Xisco Faulí 2021-01-21 21:18:05 UTC
(In reply to Jim Raykowski from comment #4)
> Direct editing is the way renaming is done in the Form Navigator. Menu >
> Form > Form Navigator... 
> 
> IMHO, it is a challenging hack for a beginning LO hacker.
> 
> If anyone is interested in taking this on please let me know. I will provide
> code  pointers and review to make this work in the Writer Navigator content
> tree.

Hi Jim,
Could you please provide the code pointers so we can turn this bug into an easyhack ?
Comment 9 Jim Raykowski 2021-01-22 08:26:37 UTC
code pointers:
 
class NavigatorTree
svx/source/inc/fmexpl.hxx
svx/source/form/navigatortree.cxx
 
IsEditingActive()
DECL_LINK(EditingEntryHdl...
DECL_LINK(EditedEntryHdl...
m_xTreeView->connect_editing(...
IMPL_LINK(NavigatorTree, EditingEntryHdl, const weld::TreeIter&, rIter, bool)
IMPL_LINK(NavigatorTree, EditedEntryHdl, const IterString&, rIterString, bool)

include/vcl/weld.hxx
class TreeView
connect_editing

class SwContentTree
sw/source/uibase/inc/conttree.hxx
sw/source/uibase/utlui/content.cxx

Add the above declarations and handlers as in the NavigatorTree to the SwContentTree.
In the EditingEntryHdl set m_bEditing according to boolean value return by lcl_IsContent

When in Editing mode always return false from SwContentTree::HasContentChanged()

void SwContentTree::ExecuteContextMenuAction(const OString& rSelectedPopupEntry)
        case 502 : // handling of popup menu entry Rename...
            EditEntry(*xFirst, EditEntryMode::RENAME);
return immediatly after calling EditEntry so UpdateListbox isn't called after breaking out of the switch

Study SwContentTree::EditEntry function, in particular the nMode == EditEntryMode::RENAME handling for the various content types.

Move rename handling out of SwContentTree::EditEntry into the newly added SwContentTree::EditedEntryHdl
This will eliminate need for CreateSwRenameXNamedDlg in SwContentTree::EditEntry

Do this by moving:
uno::Reference< container::XNameAccess >  xNameAccess;
uno::Reference< frame::XModel > xModel = m_pActiveShell->GetView().GetDocShell()->GetBaseModel();
and each X...Supplier to get xNameAccess as done in SwContentTree::EditEntry

To start in place editing use the following at each nMode == EditEntryMode::RENAME in SwContentTree::EditEntry

	m_xTreeView->start_editing(rEntry);
	return;

Renaming ContentTypeId::DRAWOBJECT is different, see: 
sw/source/uibase/shells/drwbassh.cxx
void SwDrawBaseShell::Execute
case FN_NAME_SHAPE 

To check drawing object name uniqueness see:
sw/source/uibase/shells/drwbassh.cxx
IMPL_LINK( SwDrawBaseShell, CheckGroupShapeNameHdl, AbstractSvxObjectNameDialog&, rNameDialog, bool )

Take care to do forbidden character checking for bookmark renaming and table renaming.

Rename the context menu item Rename... to Rename
sw/uiconfig/swriter/ui/navigatorcontextmenu.ui
Comment 10 Tarun Sharma 2021-05-21 10:30:49 UTC
hi jim 

> Renaming ContentTypeId::DRAWOBJECT is diff

for renaming DRAWOJBECT do i have to pass refernce of m_xTreeView, by adding a new parameter in the execute function??!
 
sw/source/uibase/shells/drwbassh.cxx


> Take care to do forbidden character checking for bookmark renaming and table renaming.

How should i validate the inputs currently its using pDlg 's method setForbiddenChars which was created from CreateSwRenameNamedDlg .....

pDlg(pFact->CreateSwRenameXNamedDlg(m_xTreeView.get(), xNamed, xNameAccess));
Comment 11 Jim Raykowski 2021-05-22 05:05:16 UTC
Hi Tarun,

(In reply to Tarun Sharma from comment #10)
> hi jim 

> for renaming DRAWOJBECT do i have to pass refernce of m_xTreeView, by adding
> a new parameter in the execute function??!
>  
> sw/source/uibase/shells/drwbassh.cxx
> 

Handling for this case will also be done in the added edit entry handler IMPL_LINK(SwContentTree, EditedEntryHdl, const IterString&, rIterString, bool)
in sw/source/uibase/utlui/content.cxx

> How should i validate the inputs currently its using pDlg 's method
> setForbiddenChars which was created from CreateSwRenameNamedDlg .....
> 
> pDlg(pFact->CreateSwRenameXNamedDlg(m_xTreeView.get(), xNamed, xNameAccess));

Check the rIterString.second for forbidden chars. For bookmark forbidden chars see: sw/source/uibase/inc/bookmark.hxx static constexpr OUStringLiteral aForbiddenChars. If found, break out of the case before setting xNameAccess.

Looking forward to seeing the patch! Please submit to gerrit and add me as a reviewer so we can discuss code there.
Comment 12 Roman Kuznetsov 2021-06-05 08:35:21 UTC
https://gerrit.libreoffice.org/c/core/+/116731
Comment 13 Commit Notification 2022-11-09 07:24:31 UTC
Jim Raykowski committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/ace75043781b5fe36546ec75574a14617f4feb30

tdf#139633 SdNavigator: Enhancement to rename pages and objects

It will be available in 7.5.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 14 Buovjaga 2022-12-04 11:20:52 UTC
Testing the new feature, it seems to do what it promises :) Anything else to be done or can this be closed?
Comment 15 Jim Raykowski 2022-12-04 16:02:08 UTC
(In reply to Buovjaga from comment #14)
> Testing the new feature, it seems to do what it promises :) Anything else to
> be done or can this be closed?

Only done for Draw/Impress Navigator. Might want to leave open for Writer and Calc?
Comment 16 Commit Notification 2023-08-16 06:02:23 UTC
Jim Raykowski committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/b363a689bf5b434113648373f98ee46aa00c09d7

tdf#156610 tdf#139633 SDNavigator: Fix object naming/renaming

It will be available in 24.2.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 17 Commit Notification 2023-08-22 17:47:02 UTC
Jim Raykowski committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/6761b79e7b48a5885adecf445f0206f141888613

tdf#156610 tdf#139633 SdNavigator: Fix object not selected

It will be available in 24.2.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 18 Stéphane Guillou (stragu) 2024-03-07 06:20:32 UTC
(unassigning, had been assigned for 2.5 years to Tarun)