maciejhirsz / kobold

Easy declarative web interfaces.
https://docs.rs/kobold/
Mozilla Public License 2.0
385 stars 7 forks source link

Explicitly specify state table property to use to prevent conflicts #53

Closed ltfschoen closed 1 year ago

ltfschoen commented 1 year ago

In the csv example, table is a property of State https://github.com/maciejhirsz/kobold/blob/master/examples/csv_editor/src/state.rs#L13 and columns and rows are properties of Table https://github.com/maciejhirsz/kobold/blob/master/examples/csv_editor/src/state.rs#L18 and we can use columns() or rows() to get them for Table https://github.com/maciejhirsz/kobold/blob/master/examples/csv_editor/src/state.rs#L81 but in main.rs, its calling state.columns() and state.rows() in places like here https://github.com/maciejhirsz/kobold/blob/master/examples/csv_editor/src/main.rs#L59, but i think it should be calling state.table.columns() and state.table.rows().

otherwise you get conflicts, like in the invoice example here where both Table and TableFileDetails have properties columns and rows, and also the methods columns() and rows(), so i have to explicitly specify like here state.table.columns() and state.table.rows(), or state.table_file_details.rows() and state.table_file_details.columns(), otherwise i might get the wrong one. for example if i just used state.columns() it'd output columns 0..2, but if i used state.table_file_details.columns() it output columns 0..10, which was what i was after

i think it should even use state.table.source instead of juststate.source

maciejhirsz commented 1 year ago

I haven't looked at it in detail, but why do you have basically the same struct with the same impls twice?

Your state should be just:

pub struct State {
    pub editing: Editing,
    pub name: String,
    pub name_file_details: String,
    pub table: Table,
    pub table_file_details: Table,
    pub entry: Entry,
    pub entry_editing: bool,
    pub qr_code: String,
}

Or better yet, since you have two tables going on and both have a name, do something like:

pub struct Content {
    pub name: String,
    pub table: Table,
}

pub struct State {
    pub editing: Editing,
    pub main: Content,
    pub details: Content,
    pub entry: Entry,
    pub entry_editing: bool,
    pub qr_code: String,
}

Then you can have Deref<Target = Table> implemented for Content, remove the one for State and you could do state.main.columns() or state.details.columns() with no ambiguity.

ltfschoen commented 1 year ago

Thanks for the tips. I actually considered only using Table but made the wrong choice at the time 👶 I tried your suggestions and they work great, I pushed changes here https://github.com/ltfschoen/kobold/commit/833b66c8d0fe8a36b8c3a2c247ba512f11ba6793

maciejhirsz commented 1 year ago

See https://github.com/maciejhirsz/kobold/issues/58#issuecomment-1496014208