ocsigen-toolkit icon indicating copy to clipboard operation
ocsigen-toolkit copied to clipboard

Add a color picker to ocsigen-toolkit

Open cedlemo opened this issue 8 years ago • 2 comments

In this issue, @vasilisp told me that you could be interested with a color picker like in ocsigen-widgets. I have started to just copy/paste the Ow_table_color_picker code and it works, here is what I have :

color_picker_test.eliom

[%%shared
    open Eliom_lib
    open Eliom_content
    open Html.D
    open Lwt
]

module Color_picker_test_app =
  Eliom_registration.App (
    struct
      let application_name = "color_picker_test"
      let global_data_path = None
    end)

let (listeners, color_selector, color_table) = Ot_color_picker.create ()

let body_content () =
  div ~a:[a_id "wrapper"]
      [
        div ~a:[a_id "left"] [color_selector];
        div ~a:[a_id "right"] [color_table]
      ]



let page () =
  html
    (head (title (pcdata "Color Picker")) [
          css_link ~uri:(make_uri (Eliom_service.static_dir ())
                           ["css";"color_picker_test.css"])
            ()]
    )
    (body [body_content ()])


let main_service =
  Color_picker_test_app.create
    ~path:(Eliom_service.Path [])
    ~meth:(Eliom_service.Get Eliom_parameter.unit)
    (fun () () ->
       let _ = [%client (Ot_color_picker.start ~%listeners : unit)] in
       Lwt.return (page ())
    )

the css :

* {
    font-family: sans-serif;
}
#wrapper {
	width: 60%;
	margin: 0 auto;
	height: 100vh;
}
#left {
	float: left;
	width: 80%;
	height: 100vh;
}
#right {
	float: left;
}
.ew_table_color_picker_square {
	height: 0.9em;
	width: 0.9em;
	border-radius:0.1em;
}

.ew_table_color_picker_color_div {
	height: 100%;
}

And an image :

screenshot

Basically, I changed nothing from the code here :

  • https://github.com/ocsigen/ocsigen-widgets/blob/master/src/widgets/ow_table_color_picker.eliomi
  • https://github.com/ocsigen/ocsigen-widgets/blob/master/src/widgets/ow_table_color_picker.eliom

Before I do the PR, do you want me to change something ?

cedlemo avatar May 20 '17 20:05 cedlemo

Hi @cedlemo! Thanks for your continued interest, and sorry for the delay.

It would be nice to clean up the code and interface instead of just copying it over.

To begin with, I would like to see

  • More descriptive names than *_lll_*. What does lll stand for? genere_ -> generate_.
  • Documentation comments (i.e., (** ... **)) for everything that matters, in proper English and formatted without the annoying *** in every line.
  • A description in the beginning of the .eliomi like for the other widgets.
  • A style in the css dir, following the naming conventions of the other widgets.

Feel free to do a PR, and we will provide feedback as needed.

vasilisp avatar May 24 '17 14:05 vasilisp

Feel free to do a PR, and we will provide feedback as needed.

Ok, I work on it.

cedlemo avatar May 24 '17 16:05 cedlemo