Re: [lmi] Direct drill-down census editor testing

[Václav Slavík]

Hi,

On 20 Oct 2011, at 14:36, Wendy Boutin wrote:
> 1. Keyboard navigation is very limited. I can use the up and down
> arrow keys to select different cells, but I can't drill down to
> any particular data column within a cell.

Yes. This is something I mentioned here before, this functionality is missing 
in the wxWidgets version used by LMI. The trunk version is much better, I just 
have some minor finishing touches to do, before it's fully ready for LMI. I'll 
post here once all the patches land.

> 2. It's natural for most users to double-click the field they want
> to modify, but that doesn't seem to work like I'd expect. The census
> editor seems more like two single-clicks instead.

That's actually the native Windows (and OS X and Linux/GTK+...) behavior. See 
e.g. Explorer, where file names are edited the same way and double-click is 
used to open files or folders. In LMI, double-click should correspond to Ctrl+E 
(and I keep getting surprised when I can't open the editor this way).

We could, of course, change CensusView to handle double-click differently from 
Explorer, but I think it's better to stick to the native behavior.

> 4. The size of input controls is inconsistent; text controls look
> shorter in height than all other controls.
...
> 5. Selecting a control for editing causes it to grow tall enough
> to cover much of the data in the row below it. It can be convenient
> to see the next cells data if a user is editing an entire column
> from top to bottom.


This should be fixed, of course -- all inline controls should use the same 
height and it should correspond to the row's height when not editing, both for 
the practical reasons you mention and for aesthetic ones.

> Also, within the text
> control, the alignment of the inputted text makes it difficult to
> read. Maybe all it needs is a border, but right now it isn't easy
> to distinguish a 'q', 'g', and 'p'.

I believe this is a result of contradictory requirements:
On one hand, we clearly want the text control to use the same size as the row 
it is on.
On the other hand, there's the requirement to make the rows small:
http://lists.nongnu.org/archive/html/lmi/2011-07/msg00041.html

I'll need to double-check this first, but if I'm right, then the best fix would 
be reverting to slightly taller rows.

> Input validation:
> 
> 6. There isn't any input validation in the census view. It really
> needs to have the same level of input validation that's currently
> in the individual cell input screens.

Yes, this is a serious problem :-(

Unfortunately, I don't know how to fix it. I couldn't find any validation code 
in the editor files when I looked, my impression was that it's handled by 
any_member<Input>::operator=(). Apparently, it isn't, what else do I need to do?

For reference, here's the offending bit of code in the new CensusView in its 
entirety (with a mark added in the place where more validation is needed):

bool CensusViewDataViewModel::SetValueByRow(wxVariant const& variant, unsigned 
row, unsigned col)
{
    LMI_ASSERT(col != Col_CellNum);

    any_member<Input>& cell = cell_at(row, col);
    renderer_type_convertor const& conv = renderer_type_convertor::get(cell);

    std::string const prev_val = cell.str();
    std::string new_val = conv.from_variant(variant);

    if(prev_val == new_val)
        return false;

    // TODO: How to validate 'new_val' here?
    cell = new_val;

    view_.document().Modify(true);

    return true;
}


Thanks,
Vaclav