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

Color picker #141

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions css/ot_color_picker.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/* It regroups one table of colors */
.ot-color-picker-table {
}

.ot-color-picker-table-td {
}

.ot-color-picker-table-tr {
}

/* A class for the div contained in each table cell. It is used to display the
* color to select. */
.ot-color-picker-square {
height: 0.9em;
width: 0.9em;
border-radius:0.1em;
}

/* The set of tables of colors */
.ot-color-picker-block {
}


/* A class for the div that displays the currently selected color. */
.ot-color-picker-current-color {
}
183 changes: 183 additions & 0 deletions src/widgets/ot_color_picker.eliom
Original file line number Diff line number Diff line change
@@ -0,0 +1,183 @@
[%%shared

open Eliom_content.Html
open Eliom_content.Html.F

type div = Html_types.div Eliom_content.Html.D.elt
type t = (string ref * div * div list * div)

let raise_color_samples_exception () =
let message = "Ot_color_picker.generate_color_samples, \
the argument have to be greater than 1" in
invalid_arg message

let generate_color_samples precision =

let color_list = match precision with
| p when p <= 1 -> raise_color_samples_exception ()
| precision ->
let step = 255 / (precision - 1) in
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

step becomes 0 for precision >= 257, with undesired consequences. If we do input validation for precision <= 1, we had better do it for the upper bound as well.

Also, using match instead of if is not a good idea in this case. It makes the code less readable by introducing new identifiers.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

let rec aux_build nl v =
if (v > 255)
then nl
else aux_build ((Printf.sprintf "%02X" v)::nl) (v + step)
in aux_build [] 0
in
let max_iteration = List.length color_list in

let rec aux_red nl = function
| n when n >= max_iteration -> nl
| n ->
let red = List.nth color_list n in
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's very ugly code.

Instead of recurring on color_list, we recur on its indices and perform a linear-cost List.nth in every step! The List.nth inside aux_blue will increase the big-O complexity without reason.

Could you try to rewrite this in terms of nested List.map invocations?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done.


let rec aux_green nl = function
| n when n >= max_iteration -> nl
| n ->
let green = List.nth color_list n in

let rec aux_blue nl = function
| n when n >= max_iteration -> nl
| n ->
let blue = List.nth color_list n in
aux_blue (("#" ^ red ^ green ^ blue)::nl) (n + 1)

in aux_green ((aux_blue [] 0)::nl) (n + 1)

in aux_red ((aux_green [] 0)::nl) (n + 1)

in aux_red [] 0


(* Some pre-generated color samples in several precisions. *)
let color_samples_p2 = lazy (generate_color_samples 2)
let color_samples_p3 = lazy (generate_color_samples 3)
let color_samples_p4 = lazy (generate_color_samples 4)
let color_samples_p5 = lazy (generate_color_samples 5)
let color_samples_p6 = lazy (generate_color_samples 6)

(* Some hand-mained color samples *)
let color_samples_10 = [[["#E03625"; "#FF4B3A"];
["#FF7E02"; "#FFC503"];
["#01CD64"; "#AF58B9"];
["#0198DD"; "#254760"];
["#FFFFFF"; "#000000"]]]

let color_samples_6 = [[["#BEC3C7"; "#7A8E8D"];
["#1C3D50"; "#0280B4"];
["#00A385"; "#A444B2"]]]


(* Take one list of list of list of color (string) and build table list with it.
return also div_color_list to allow to launch start script detection. *)
let generate_color_table color_samples =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Takes a list of lists of lists of colors (strings) and returns a table list. Also returns a div_color_list for launching start script detection.

The singular/plural needs to be fixed in the .eliomi.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


let build_color_div color =
D.div ~a:[a_class["ot-color-picker-square"];
a_title color;
a_style ("background-color: " ^ color ^ ";")]
[]
in
let build_td_color color_div =
td ~a:[a_class["ot-color-picker-table-td"]] [color_div]
in
let build_tr_color tds =
tr ~a:[a_class["ot-color-picker-table-tr"]] tds
in

let rec build_table div_color_list tables = function
| [] -> div_color_list, tables
| head::tail ->

let rec build_column div_color_list trs = function
| [] -> div_color_list, trs
| head::tail ->

let rec build_line div_color_list tds = function
| [] -> div_color_list, tds
| color::tail ->

let color_div = build_color_div color in
build_line
(color_div::div_color_list)
((build_td_color color_div)::tds)
tail
in

let div_color_list', tds = build_line div_color_list [] head in
build_column
div_color_list'
((build_tr_color tds)::trs)
tail
in

let div_color_list', trs = build_column div_color_list [] head in
let tbl = table ~a:[a_class["ot-color-picker-table"]] trs in
build_table
div_color_list'
(tbl::tables)
tail
in

let div_color_list, tables = build_table [] [] color_samples in
div_color_list, tables

let make ?(initial_color = 0, 0, 0) ?(color_samples = Lazy.force color_samples_p5) () =
let tbl, trl, tdl = initial_color in
let color_ref = ref (List.nth (List.nth (List.nth color_samples tbl) trl) tdl) in
let div_color_list, tables = generate_color_table color_samples in
let color_div = D.div ~a:[a_class["ot-color-picker-current-color"];
a_title !color_ref;
a_style ("background-color: " ^ !color_ref ^ ";")] []
in
let block = D.div ~a:[a_class["ot-color-picker-block"]] tables in
let type_t = (color_ref, color_div, div_color_list, block) in
type_t, color_div, block

]

[%%client

open Lwt

let fusion (color_ref, color_div, fst_list, block) (_, _, snd_list, _) =
(color_ref, color_div, fst_list@snd_list, block)

let start (color_ref, color_div, color_list, _) =
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can start happen automatically? I don't think we have similar start methods in the other Ocsigen Toolkit widgets.

let dom_color_div = Eliom_content.Html.To_dom.of_element color_div in
let rec aux = function
| [] -> ()
| div_elt::tail ->
let dom_div = Eliom_content.Html.To_dom.of_element div_elt in
Lwt.async (fun () ->
Lwt_js_events.clicks dom_div (fun _ _ ->
Lwt.return
(let color = dom_div##.title in
dom_color_div##.style##.backgroundColor := color;
dom_color_div##.title := color;
color_ref := (Js.to_string color))));
aux tail
in aux color_list

let generate_and_append (color_ref, color_div, fst_list, block) new_list =
let div_color_list, tables = generate_color_table new_list in
let aux = function
| tbl::t -> Eliom_content.Html.Manip.appendChild block tbl
| [] -> ()
in aux tables;
div_color_list

let add_square_color color_picker new_list =
let color_ref, color_div, fst_list, block = color_picker in
color_ref, color_div,
fst_list@(generate_and_append color_picker new_list), block

let add_square_color_and_start color_picker new_list =
let color_ref, color_div, fst_list, block = color_picker in
ignore (start (color_ref, color_div,
generate_and_append color_picker new_list, block))

let get_color (color_ref, _ , _, _) = !color_ref

let get_square_color_div_list (_, _, color_list, _) = color_list

]
70 changes: 70 additions & 0 deletions src/widgets/ot_color_picker.eliomi
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
[%%shared.start]

(** Ot_color_picker : this module allows to generate, client side, a color
selector. This selector consists of a color table and an html div that
displays the current selected color. *)

(** The main type of Ot_color_picker module *)
type t
type div = Html_types.div Eliom_content.Html.D.elt

(** The argument is the divisor of 255. It have to be greater than 1 *)
val generate_color_samples : int -> string list list list

(* Some pre-generated color samples in several precision. Color samples
are list of list of list of colors represented in string of hexadecimal
values.*)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need (** for this to appear in the doc.

You should probably leave a space before color_samples_p2 so that the comment refers to all the color_samples_p* that follow.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

val color_samples_p2 : string list list list Lazy.t
val color_samples_p3 : string list list list Lazy.t
val color_samples_p4 : string list list list Lazy.t
val color_samples_p5 : string list list list Lazy.t
val color_samples_p6 : string list list list Lazy.t

(* Some hand-mained color samples. *)
val color_samples_6 : string list list list (* 1 table 2 columns 5 lines *)
val color_samples_10 : string list list list (* 1 table 2 columns 3 lines *)

(** Take one list (tables) of list (columns) of list (lines) of color (string)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Attention to the plurals.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

and build the table of colors with it.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

builds

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

By default this list is initialised with color_samples_p5

It return
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

returns

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

- t to future action,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for future actions (?)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

- color_div, to display current select color,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the currently selected color

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

it is not mandatory to include it in page
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in the page

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

- and the block with all color square content in the generated table *)
val make :
?initial_color: int * int * int ->
?color_samples: string list list list ->
unit ->
(t * div * div)



[%%client.start]

(** Get two color_picker to fusion in a single one. This new color_picker uses
color squares of both.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

color pickers

the color squares of both

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

It uses color_div of first color_picker given in argument. It also keeps
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The color_div of the first color_picker

referend on first color_picker's block to future append color
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

keeps a reference to the first color_picker's block for appending a color in the future (?).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


This action has to be done before using the start function *)
val fusion : t -> t -> t

(** It allows to add square color and to append directly in block
It has to be made before start *)
val add_square_color : t -> string list list list -> t

(** Launch listeners *)
val start : t -> unit

(** It allows to add square color after the start function. It have not to be
used before start *)
val add_square_color_and_start : t -> string list list list -> unit
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't say I understand the logic of add_square_color*. Why do we have two of them? If the widgets has to be configurable with an extra component, can this happen via an optional argument to make?


(** Get the currently selected color of the selector. The fusion or add_square
functions have no effects on it. *)
val get_color: t -> string
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We generally use React.t signals for outputs like this.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@vasilisp , sorry for the delay (I am an absolute beginner so I try to figure out how it works).

If I well understand, I should in fact modify the make function to make it returns something like that :

(t * div * div * string Eliom_shared.React.S.t)

where string Eliom_shared.React.S.t is a signal with the currently selected color like in the ot_time_picker ?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's what I have in mind. Maybe string option Eliom_shared.React.S.t, with the value of the signal being None before the user picks a color. Give it a try if you are likewise motivated :).


(** get all square color div element *)
val get_square_color_div_list : t -> div list