Re: [Libreoffice-ux-advise] [Libreoffice] [PATCH] fdo#45285 - Use more revealing error message for not allowed sheet names
Hi,
> Hi Albert,
>
>> ... The sheet name must not be a duplicate of an existing name.
>
> Yes.
>
>> ... Further it must be 31 or fewer characters and can not contain the >
>> characters [ ] * ? / \ .
>
> No, AFAIK there is no " <= 31" restriction. And '.' isn't a forbidden
> character, see the source code:
> http://cgit.freedesktop.org/libreoffice/core/tree/sc/source/core/data/document.cxx#n250
>
> But anyway improving this error message would be great! :) I believe that
> most people hate meaningless error messages.
>
> Thanks,
> Ivan
Thanks for the feed-back, Ivan! I have updated the patch accordingly.
/Albert
From 31dd0f0a8c1b280d4958cf75b2090dd2faa2579b Mon Sep 17 00:00:00 2001
From: Albert Thuswaldner
Date: Sat, 28 Jan 2012 20:54:09 +0100
Subject: [PATCH] Updated error message for Invalid sheet name
---
sc/source/ui/src/globstr.src |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/sc/source/ui/src/globstr.src b/sc/source/ui/src/globstr.src
index 20ab9aa..6c87a30 100644
--- a/sc/source/ui/src/globstr.src
+++ b/sc/source/ui/src/globstr.src
@@ -687,8 +687,7 @@ Resource RID_GLOBSTR
};
String STR_INVALIDTABNAME
{
-Text [ en-US ] = "Invalid sheet name." ;
+Text [ en-US ] = "Invalid sheet name. \nThe sheet name must not be a duplicate of an existing name \nand can not contain the characters [ ] * ? / \\ " ;
};
String STR_SCENARIO
{
--
1.7.3.4
___
Libreoffice-ux-advise mailing list
Libreoffice-ux-advise@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice-ux-advise
Re: [Libreoffice-ux-advise] [Libreoffice] [PATCH] [PUSHED] fdo#45285 - Use more revealing error message for not allowed sheet names
Hi, On Mon, Jan 30, 2012 at 10:07, Stefan Knorr (Astron) wrote: > Hi Albert, Korrawit, Ivan, > > the idea sounds good, ... patch is simple, and the details seem to be > correct now. Thus, I've taken the liberty to replace "can" by "may" > and then push to master. > http://cgit.freedesktop.org/libreoffice/core/commit/?id=e70541dc4627d63f294837f3eefc1cc25398b703 Thanks for taking care of this (and yes "may" instead of "can" sounds better). Just a thought: should we push this to the 3.5 branch? This is a small polish of the user experience and it might be good to have it in a release soon, instead of waiting for 3.6. Also the bug report was written against 3.5.0rc1... > Also, thanks for the patch. > > Albert, could you please confirm that you submitted your patch under > LGPL3+/MPL? (If you plan on doing more patches, please consider just > writing a blanket mail saying that you want to always commit under > these licenses and then add yourself to [1].) Done. /Albert ___ Libreoffice-ux-advise mailing list Libreoffice-ux-advise@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice-ux-advise
Re: [Libreoffice-ux-advise] [Libreoffice] [PATCH] [PUSHED] fdo#45285 - Use more revealing error message for not allowed sheet names
Hi, > Well - hopefully six months away is not -so- far - but I appreciate > your pain. Of course - in theory with triple review features can get > into the -3-5 branch, but string changes are less welcome I suspect. > Well, for me it is not that important. I just wanted to make sure before I told the guy who reported the bug that it wouldn't make it into 3.5.x. /Albert ___ Libreoffice-ux-advise mailing list Libreoffice-ux-advise@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/libreoffice-ux-advise
Re: [Libreoffice-ux-advise] [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
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 mpOldOptions;
::boost::shared_ptr 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
-
#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()
Re: [Libreoffice-ux-advise] [Libreoffice] [PATCH] Making default tab prefix name configurable in Calc
On Mon, Feb 13, 2012 at 12:23, Noel Power wrote:
> On 09/02/12 14:53, Albert Thuswaldner wrote:
>>> 2) When trying to delete text in the edit box, the last character
>>> sticks (can't be deleted).
>
> well that would seem to be because your code thinks a blank prefix is
> illegal, I believe changing
>
> - if ( !ScDocument::ValidTabName( aSheetPrefix ) )
> + if ( !aSheetPrefix.isEmpty() && !ScDocument::ValidTabName( aSheetPrefix
> ) )
> {
> // Revert to last good Prefix
> pEdit->SetText( maOldPrefixValue );
> }
>
> will fix that
>
Agggh, your right :)
On Mon, Feb 13, 2012 at 13:11, Noel Power wrote:
> On 09/02/12 14:53, Albert Thuswaldner wrote:
>
> If it was up to me I would go for the visual bell. Any thoughts on this?
>
> actually thinking about it some more I don't find it unnatural to have the
> whole ( reverted ) prefix selected after entering an illegal character ( I
> think this is a technique used elsewhere anyway ) In addition to providing a
> cue that you have typed something bad this approach has the added advantage
> of solving the cursor problem for you too ;-)
>
> e.g.
>
> // Revert to last good Prefix
> Selection aSel( 0, maOldPrefixValue.getLength() );
> pEdit->SetText( maOldPrefixValue, aSel );
>
> Not sure what others think about above but if I hear no real negative
> reaction then I'd be inclined to push this
>
> p.s. of course you can use the 'Selection'
> Selection aSel( maOldPrefixValue.getLength(),
> maOldPrefixValue.getLength() )
>
> to trick SetText to display the cursor at the end. This is a bit hacky, the
> alternative is you will probably need to modify the cursor position
> directly, that is, unless there is some existing public method on Edit (
> which there might be but I didn't see in the myriad of 'Edit' methods )
Yes I vote for pushing this as you suggested. It is a cheap solution
for now, and done elsewhere in the code, so at least we are consistent
then.
Saw the ideas from Markus and Stefan regarding the range name dialog:
https://wiki.documentfoundation.org/Design/Whiteboard/Calc_Range_Names
A mode-less window (i.e balloon) would be an option for a future
improvement. But I guess we leave that for now until we have a
consensus on going down that route.
Noel: can you push the patch with your changes or do you want me to tweek it?
/Albert
___
Libreoffice-ux-advise mailing list
Libreoffice-ux-advise@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/libreoffice-ux-advise
