Skip to content
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

Add a color picker to ocsigen-toolkit #140

Open
cedlemo opened this issue May 20, 2017 · 2 comments
Open

Add a color picker to ocsigen-toolkit #140

cedlemo opened this issue May 20, 2017 · 2 comments

Comments

@cedlemo
Copy link

cedlemo commented May 20, 2017

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 :

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

@vasilisp
Copy link
Contributor

vasilisp commented May 24, 2017

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.

@cedlemo
Copy link
Author

cedlemo commented May 24, 2017

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

Ok, I work on it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants