Re: [Libreoffice] [PATCH] Making default tab prefix name configurable in Calc

Hi,

> Given that, I still think it's better (and easier) to check for every
> key-press, but instead of launching an error dialog for an illegal name,
> just put the old legal name back into the text box and sound an beep or
> something.  That's less intrusive than a big dialog being thrown right
> in your face.  I do exactly that for the argument separator value inputs
> in the Formula option page.

So I have improved the patch according to Kohei's suggestion above.
Now I store the prefix at each key-stroke and check for illegal
characters. When an illegal character is entered, the text is reverted
back to the last good prefix. (So basically the same as for the
separators in the formula options dialog)

The patch also fixes the UI glitches.

So it all should be well - but it is not!

At least I get two annoying bugs:
1) The cursor moves In front of the text when the Prefix is reverted.
2) When trying to delete text in the edit box, the last character
sticks (can't be deleted).

Does someone have a clue whats going on?

To finish this feature I also would like to add some type of cue.  It
would be great if there would be some kind of standard to follow here
(is there?).

Below I just list the different options (including possible pros/cons)
that I can think of at the moment:

1) Error dialog
+ Grabs attention
+ Space to provide a meaningful error message.
- too in your face

2) FixedLine message
+ Not in-your-face.
- Takes up space
- Only space for short error message

3) System bell
+ Grabs attention
- Utterly annoying IMO

4) Visual bell (tinted red flashing of edit field)
+ Subtile and effective way of feedback.
- ?

5.) Icon (warning sign or exclamation mark)
+ Can be made click-able to give additional information.
- Takes up space.

6.) Balloon message
+ ?
- ?

If it was up to me I would go for the visual bell. Any thoughts on this?

Thanks for reading my ramblings. ;)
/Albert

From c4e78f3a3c59c76a50639d990803f0edf7f1c4b4 Mon Sep 17 00:00:00 2001
From: Albert Thuswaldner <albert.thuswaldner@gmail.com>
Date: Thu, 9 Feb 2012 15:26:27 +0100
Subject: [PATCH] Improvment of Custom Sheet Prefix Option

---
 sc/source/ui/inc/tpdefaults.hxx    |    7 ++++-
 sc/source/ui/optdlg/tpdefaults.cxx |   45 +++++++++++++++++++++++++++--------
 sc/source/ui/src/optdlg.src        |    6 ++--
 3 files changed, 43 insertions(+), 15 deletions(-)

diff --git a/sc/source/ui/inc/tpdefaults.hxx b/sc/source/ui/inc/tpdefaults.hxx
index 606d174..7369988 100644
--- a/sc/source/ui/inc/tpdefaults.hxx
+++ b/sc/source/ui/inc/tpdefaults.hxx
@@ -53,10 +53,12 @@ private:
     virtual ~ScTpDefaultsOptions();
 
     void CheckNumSheets();
-    void CheckPrefix();
+    void CheckPrefix(Edit* pEdit);
+    void OnFocusPrefixInput(Edit* pEdit);
 
     DECL_LINK( NumModifiedHdl, NumericField* );
     DECL_LINK( PrefixModifiedHdl, Edit* );
+    DECL_LINK( PrefixEditOnFocusHdl, Edit* );
 
 private:
     FixedLine     aFLInitSpreadSheet;
@@ -65,6 +67,9 @@ private:
     FixedText     aFtSheetPrefix;
     Edit          aEdSheetPrefix;
 
+    // Stores old Sheet Prefix
+    ::rtl::OUString maOldPrefixValue;
+
     ::boost::shared_ptr<ScDocOptions> mpOldOptions;
     ::boost::shared_ptr<ScDocOptions> mpNewOptions;
 };
diff --git a/sc/source/ui/optdlg/tpdefaults.cxx b/sc/source/ui/optdlg/tpdefaults.cxx
index 8c97c83..ec29550 100644
--- a/sc/source/ui/optdlg/tpdefaults.cxx
+++ b/sc/source/ui/optdlg/tpdefaults.cxx
@@ -29,8 +29,6 @@
 
 #undef SC_DLLIMPLEMENTATION
 
-#include <vcl/msgbox.hxx>
-
 #include "tpdefaults.hxx"
 #include "optdlg.hrc"
 #include "scresid.hxx"
@@ -49,9 +47,9 @@ ScTpDefaultsOptions::ScTpDefaultsOptions(Window *pParent, const SfxItemSet &rCor
     SfxTabPage(pParent, ScResId(RID_SCPAGE_DEFAULTS), rCoreAttrs),
     aFLInitSpreadSheet ( this, ScResId( FL_INIT_SPREADSHEET ) ),
     aFtNSheets         ( this, ScResId( FT_NSHEETS ) ),
-    aEdNSheets                    ( this, ScResId( ED_NSHEETS ) ),
-    aFtSheetPrefix        ( this, ScResId( FT_SHEETPREFIX ) ),
-    aEdSheetPrefix        ( this, ScResId( ED_SHEETPREFIX ) )
+    aEdNSheets         ( this, ScResId( ED_NSHEETS ) ),
+    aFtSheetPrefix     ( this, ScResId( FT_SHEETPREFIX ) ),
+    aEdSheetPrefix     ( this, ScResId( ED_SHEETPREFIX ) )
 {
     FreeResource();
 
@@ -70,9 +68,10 @@ ScTpDefaultsOptions::ScTpDefaultsOptions(Window *pParent, const SfxItemSet &rCor
         Point aNewPoint = aEdNSheets.GetPosPixel();
         aNewPoint.X() += ( nTxtW - nCtrlW );
         aEdNSheets.SetPosPixel( aNewPoint );
-    }
+   }
     aEdNSheets.SetModifyHdl( LINK(this, ScTpDefaultsOptions, NumModifiedHdl) );
     aEdSheetPrefix.SetModifyHdl( LINK(this, ScTpDefaultsOptions, PrefixModifiedHdl) );
+    aEdSheetPrefix.SetGetFocusHdl( LINK(this, ScTpDefaultsOptions, PrefixEditOnFocusHdl) );
 }
 
 ScTpDefaultsOptions::~ScTpDefaultsOptions()
@@ -121,14 +120,31 @@ void ScTpDefaultsOptions::CheckNumSheets()
         aEdNSheets.SetValue(INIT_SHEETS_MIN);
 }
 
-void ScTpDefaultsOptions::CheckPrefix()
+void ScTpDefaultsOptions::CheckPrefix(Edit* pEdit)
 {
-    OUString aSheetPrefix = aEdSheetPrefix.GetText();
+    if (!pEdit)
+        return;
+
+    OUString aSheetPrefix = pEdit->GetText();
 
     if ( !ScDocument::ValidTabName( aSheetPrefix ) )
     {
-         ErrorBox(this,WinBits(WB_OK|WB_DEF_OK), ScGlobal::GetRscString(STR_INVALIDTABNAME) 
).Execute();
+        // Revert to last good Prefix
+        pEdit->SetText( maOldPrefixValue );
     }
+    else
+    {
+        OnFocusPrefixInput(pEdit);
+    }
+}
+
+void ScTpDefaultsOptions::OnFocusPrefixInput(Edit* pEdit)
+{
+    if (!pEdit)
+        return;
+
+    // Store Prefix in case we need to revert
+    maOldPrefixValue = pEdit->GetText();
 }
 
 
@@ -138,10 +154,17 @@ IMPL_LINK( ScTpDefaultsOptions, NumModifiedHdl, NumericField*, EMPTYARG )
     return 0;
 }
 
-IMPL_LINK( ScTpDefaultsOptions, PrefixModifiedHdl, Edit*, EMPTYARG )
+IMPL_LINK( ScTpDefaultsOptions, PrefixModifiedHdl, Edit*, pEdit )
+{
+    CheckPrefix(pEdit);
+    return 0;
+}
+
+IMPL_LINK( ScTpDefaultsOptions, PrefixEditOnFocusHdl, Edit*, pEdit )
 {
-    CheckPrefix();
+    OnFocusPrefixInput(pEdit);
     return 0;
 }
 
+
 /* vim:set shiftwidth=4 softtabstop=4 expandtab: */
diff --git a/sc/source/ui/src/optdlg.src b/sc/source/ui/src/optdlg.src
index edbf4b9..013c476 100644
--- a/sc/source/ui/src/optdlg.src
+++ b/sc/source/ui/src/optdlg.src
@@ -943,14 +943,14 @@ TabPage RID_SCPAGE_DEFAULTS
     FixedText FT_SHEETPREFIX
     {
         Pos = MAP_APPFONT ( 12 , 32 ) ;
-        Size = MAP_APPFONT ( 120 , 8 ) ;
+        Size = MAP_APPFONT ( 110 , 8 ) ;
         Text [ en-US ] = "Prefix name for new worksheet";
     };
     Edit ED_SHEETPREFIX
     {
         HelpID = "sc:Edit:RID_SCPAGE_CALC:ED_SHEETPREFIX";
         Border = TRUE ;
-        Pos = MAP_APPFONT ( 130 , 32 ) ;
-        Size = MAP_APPFONT ( 45 , 12 ) ;
+        Pos = MAP_APPFONT ( 130 , 30 ) ;
+        Size = MAP_APPFONT ( 60 , 12 ) ;
     };
 };
-- 
1.7.3.4