Skip to content

Add custom start week to date picker #5

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

Open
wants to merge 23 commits into
base: master
Choose a base branch
from

Conversation

fabricioc8
Copy link

@fabricioc8 fabricioc8 commented Jun 30, 2025

Preliminary steps

The following are some of the steps I made to have the repo working on my machine... (ignore them if you are only interested in the implementation itself):

  • Error running lein figwheel basic:
    Exception in thread "main" java.lang.ClassNotFoundException: javax.xml.bind.DatatypeConverter, compiling:(cljs/closure.clj:1:1)
    Caused by: java.lang.ClassNotFoundException: javax.xml.bind.DatatypeConverter

  • Diagnostic:
    The class doesn't exsist in modern java versions

Ask current java version:
~/Desktop/clojure/om-widgets> java -version
openjdk version "18.0.1.1" 2022-04-22
OpenJDK Runtime Environment (build 18.0.1.1+2-6)
OpenJDK 64-Bit Server VM (build 18.0.1.1+2-6, mixed mode, sharing)

Required version by repo: < 9

  • Solution:
    Install sdk for java version management
    ~/Desktop/clojure/om-widgets> curl -s "https://get.sdkman.io" | bash
    ~/Desktop/clojure/om-widgets> source "/Users/fabriciocozzarolo/.sdkman/bin/sdkman-init.sh"

Install and use required version
~/Desktop/clojure/om-widgets> sdk install java 8.0.452-librca

Check current version
~/Desktop/clojure/om-widgets> sdk current java
Using java version 8.0.452-librca


  • Error running lein figwheel basic:
    Exception in thread "main" java.io.FileNotFoundException: Could not locate clojure/tools/nrepl/server__init.class or clojure/tools/nrepl/server.clj on classpath., compiling:(figwheel_sidecar/repl.clj:1:1)
    Caused by: java.io.FileNotFoundException: Could not locate clojure/tools/nrepl/server__init.class or clojure/tools/nrepl/server.clj on classpath.

  • Diagnostic
    Missing dependency in classpath

  • Solution
    Add dependency to project.clj in :dependencies key [...
    [org.clojure/tools.nrepl "0.2.13"]
    ...]


  • Problem:
    Page not found in browser
    Documentation at readme file suggested navigating to localhost:3449 but figwheel server port was actually configured at
    3450 in project.clj config file

Summary of changes:

  • Datepicker:
    • Add :week-start option to control the starting day of the week (Monday or Sunday)
    • Fix bug generating negative month numbers
    • Modify days-building fn logic to fit new behavior
  • Inputtext:
    • Improve readability with more idiomatic expressions
    • Add :date-format option to control mask format
    • Validate :date-format values
    • Calculate placeholder based on :date-format to avoid inconsistencies
    • Fix placeholder when no value is set (now placeholder is shown properly)
    • Modify convert-input and convert-output fns to handle conversions between private-state and app-state
  • Add init values to example app-state
  • Add calls to modified components in examples

Tester info

  • Datpicker:

    • verify number of the days and columns position are ok when changing between 2 layouts
  • Textinput:

    • remove initial value from app-stare example: only placeholder should be shown
    • changing the value in the input should change value in date picker
    • changing the value in datepicker should modify input value
    • the format of the date should change according the value of :date-format value provided when calling the component

days-to-fill (range (inc (- last-day (dec weekday-current-month))) (inc last-day))]
days-to-fill (range (inc
(- last-day
(case week-start
Copy link
Contributor

Choose a reason for hiding this comment

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

que problema le ves a esto?

Copy link
Author

@fabricioc8 fabricioc8 Jul 2, 2025

Choose a reason for hiding this comment

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

mm te referis al uso de case? o notas un bug en el calculo?

Copy link
Author

@fabricioc8 fabricioc8 Jul 2, 2025

Choose a reason for hiding this comment

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

si es por el case, no tiene valor por default lo que causario un error por el nil. No se lo agregue porque la funcion principal ya tiene un default con el :or. Pero podria agregarse para hacerlo mas robusto supongo

Copy link
Contributor

@kernelp4nic kernelp4nic Jul 2, 2025

Choose a reason for hiding this comment

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

claro, si paso nil va a funcionar, pero si paso algo que no es :sunday o :monday va a reventar, yo esperaría o que defaultea a :monday que es el 90% de los casos, o que directamente te avise que la inicialización está mal y que los valores esperados son :monday :sunday (usar otra cosa ya es extremadamente raro)

weekday-current-month (time/day-of-week current-month)
previous-month (time/minus current-month (time/months 1))
(defn- build-previous-month-days [date week-start]
(let [current_month (time/month date)
Copy link
Contributor

Choose a reason for hiding this comment

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

ojo que snake case no se usa en clj

Copy link
Author

Choose a reason for hiding this comment

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

si... fue un typo...

(= input-format "integer") "right"
(= input-format "currency") "right"
:else "left"))}))}))
(let [valid-date-formats #{"MM/dd/yyy" "dd/MM/yyyy"}]
Copy link
Contributor

Choose a reason for hiding this comment

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

particularmente creo que llevaría este let más cerca de donde se usa y no en la raíz del componente (porque en general algo a ese nivel asumimos que va a ser usado por todos lados pero no es el caso)

Copy link
Author

Choose a reason for hiding this comment

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

sisi, es verdad, podria moverse tranquilamente a la llave :date-format

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

Successfully merging this pull request may close these issues.

2 participants