HellSpawner icon indicating copy to clipboard operation
HellSpawner copied to clipboard

hswidget/PaletteSelectWidget: refactor

Open gucio321 opened this issue 4 years ago • 5 comments

This PR contains a refactor of palette select (for dc6, dcc and d61 editors) related code.

CHANGELOG:

  • [x] fix #299
    • [x] fix bug fix bug, when palette selector wasn't able to change palette
  • [ ] remove unnecessary rgba field from dc6 widget state
  • [ ] decide, if palette should be saved in as part of editor state
  • [ ] rewrite dcc, dc6 and dt1 widget: palette shouldn't be an argument to Create. I suppose, it'd be better to add a separated method for setting this argument.

gucio321 avatar Jun 07 '21 09:06 gucio321

please rebase off of upstream master

gravestench avatar Jul 04 '21 00:07 gravestench

@gravestench done

gucio321 avatar Jul 04 '21 18:07 gucio321

okey, after a small linvestigation, I found that the bug is caused by build method improvement:

if e.selectPalette {
      // build palette select
}

        dt1Viewer := dt1widget.Create(e.state, e.palette, e.textu
        e.Layout(g.Layout{
                dt1Viewer,
        })

so building widget also happens if selectPalette is active IMO this solution is good and widgets should detect fact of chaning palette I'll put the palette field in widgetState structure, but It'll require a small edits of 3 widgets. I'll work on it soon.

gucio321 avatar Jul 16 '21 17:07 gucio321

I think that your proposed changes, which are related to selecting a palette, would be more coherent inside of this refactor.

gravestench avatar Jul 18 '21 06:07 gravestench

yeah, that's what I mean exactly, I'll work on it before this PR will be ready

gucio321 avatar Jul 18 '21 08:07 gucio321