search

jroimartin / gocui

Minimalist Go package aimed at creating Console User Interfaces.
BSD 3-Clause "New" or "Revised" License
9.92k stars 608 forks source link

Active color for view frame #62

Closed heppu closed 8 years ago

heppu commented 8 years ago

Add support for using different color for frame of currentView by new Attribute called ActiveColor. The behavior is similar to BgColor in Gui and View struct. By default the ActiveColor is set to ColorWhite so that everything looks same as without setting it.

For coloring frames without breaking the current API I added SetRuneWIthColor function which takes also the BG and FG colors as parameters. To fully benefit from this feature I added SetCurrentViewOnTop function which combines SetCurrentView and SetViewOnTop functions so that we don't need to loop views through twice. This is related to use case where the views are combined in tmux style, with shared frame lines.

_examples/activation.go demonstrates the usage of setting ActiveColor for Gui and View. asciicast

jroimartin commented 8 years ago

I applied some changes and pushed them to branch heppu-active. The most relevant ones are:

  1. I merged *Gui.SetRuneWithColor with *Gui.SetRune. I know it breaks the API, but we are in v0.3 and I feel it's better to put consistency first. For instance, this change makes *Gui.SetRune consistent with *View.setRune.
  2. I removed *Gui.SetCurrentViewOnTop. It forces you to loop twice over the views, but the performance penalty shouldn't be a problem (won't be called continuously) and it keeps the API simple.
  3. *Gui.SetCurrentView returns (*View, error), just like *Gui.SetViewOnTop.
  4. Minor style changes.
  5. I updated _examples/dynamic.go to use ActiveColor (I like it :D).

What do you think?

Thanks a lot for your contribution!

heppu commented 8 years ago
  1. I thought the same but was afraid that you might not feel the same about breaking the current API.
  2. Makes sense and simplifies the API like you said.
  3. I was thinking exactly the same thing when going through the source.

Everything looks good to me. =)

heppu commented 8 years ago

@jroimartin I applied the changes to this merge request so that it can be merged.

jroimartin commented 8 years ago

@heppu I've merged the branch heppu-active because I had already squashed your commits and it was easier to do fast-forward. I hope you don't mind :)

heppu commented 8 years ago

Not at all thanks for great library =)

jroimartin commented 8 years ago

Cool! Could you give a look at #63? It's kinda related :)

jroimartin commented 8 years ago

I've just realized that the original behaviour of *Gui.{FgColor,BgColor} was modified. Originally, these fields were used to define the color of the GUI (frames), but with these changes it stopped working.

So, I pushed the following changes c0ae0719314996cfc7f888f87896b35b748aa3e2, a7019c85474edf609ada2c24805cbf1c91651f9c:

I think that using Gui.{SelFgColor,SelBgColor} to set current view's color is more consistent with View.{SelFgColor,SelBgColor}, which is used for line highlighting.