New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement UI::Configuration so users can customize Flipper UI text #306

Merged
merged 1 commit into from Nov 21, 2017

Conversation

Projects
None yet
2 participants
@AlexWheeler
Collaborator

AlexWheeler commented Nov 17, 2017

spike on #63 to allow users to customize the Flipper UI text, curious what everyone thinks.

Flipper::UI.configure do |c|
  c.percentage_of_actors.title = "% of Actors"
  c.percentage_of_actors.description = "percentage of actors rule :)"

  c.actors.description = "Enable any actor by entering that actor's flipper_id"

  c.delete.title = "DO NOT PRESS!"
  c.delete.description = "Who knows what will happen?!"
end

results in:
screen shot 2017-11-17 at 8 19 50 am

I also played with the idea of making the configuration have a bit cleaner dsl something like. the latter is cleaner, but the above is simpler, would be cool to hear thoughts on this:

Flipper::UI.configure do |c|

  c.actors do
    title ""
    description ""
  end
  
end
@jnunemaker

I like it! I'm 100% down with shipping this as is. Some thoughts for this PR or a follow up:

  • feature_creation_enabled and application_breadcrumb_href - Should we move these into UI::Configuration? If we wanted backwards compatibility we could even make the current writer methods just update the configuration instance.
@jnunemaker

This comment has been minimized.

Owner

jnunemaker commented Nov 17, 2017

I hit submit too soon. Lol.

I also played with the idea of making the configuration have a bit cleaner dsl something like. the latter is cleaner, but the above is simpler, would be cool to hear thoughts on this:

I like the way you did it. I'm a fan of pure/boring ruby over DSL's.

@AlexWheeler AlexWheeler force-pushed the ui-configuration branch from 2cec712 to 3deef8e Nov 17, 2017

@AlexWheeler

This comment has been minimized.

Collaborator

AlexWheeler commented Nov 18, 2017

@jnunemaker great idea, I'll add move feature_creation_enabled / application_breadcrumb_href to UI::Configuration in next PR.

what do you think about the failing line length code climate issues for the strings. splitting them into multi lines to make code climate happy makes it messier imo

@AlexWheeler AlexWheeler force-pushed the ui-configuration branch from 3deef8e to 504fa91 Nov 18, 2017

@jnunemaker

This comment has been minimized.

Owner

jnunemaker commented Nov 18, 2017

@AlexWheeler AlexWheeler force-pushed the ui-configuration branch from 504fa91 to 3ced5fb Nov 21, 2017

@AlexWheeler AlexWheeler merged commit 0e9f3e3 into master Nov 21, 2017

0 of 2 checks passed

continuous-integration/travis-ci/pr The Travis CI build failed
Details
continuous-integration/travis-ci/push The Travis CI build failed
Details

@AlexWheeler AlexWheeler deleted the ui-configuration branch Nov 21, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment