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

Adding flexible day calendar #379

Merged
merged 2 commits into from
Oct 3, 2020
Merged

Conversation

araujoarthur0
Copy link
Collaborator

Related issue

Closes #369

Context / Background

This PR introduces a Day Calendar with a flexible number of entries :)

What change is being introduced by this PR?

  • New button on Flexible Month Calendar to switch to Day Calendar
    flex1

  • Changes on preferences to allow flexible + day

  • New FlexibleDayCalendar class built in the same idea of the FixedDayCalendar, but with the structure of FlexibleMonthCalendar

  • Entry pairs are added on new rows in the table, with intervals between them

  • To add an entry there is a button near the day total box

  • To remove the last entry there is a dialog to confirm removal (button appears when there are more than 2 pairs of entries)

  • Input errors color the entry row with the error
    flex2

How will this be tested?

New tests were added following the idea of the other Calendar tests.

@codecov
Copy link

codecov bot commented Oct 1, 2020

Codecov Report

Merging #379 into main will increase coverage by 1.51%.
The diff coverage is 63.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #379      +/-   ##
==========================================
+ Coverage   55.21%   56.72%   +1.51%     
==========================================
  Files          24       25       +1     
  Lines        2063     2373     +310     
  Branches      348      401      +53     
==========================================
+ Hits         1139     1346     +207     
- Misses        818      908      +90     
- Partials      106      119      +13     
Impacted Files Coverage Δ
src/preferences.js 0.00% <ø> (ø)
js/classes/FlexibleMonthCalendar.js 54.40% <37.50%> (-0.10%) ⬇️
js/classes/FlexibleDayCalendar.js 65.78% <65.78%> (ø)
js/classes/CalendarFactory.js 66.66% <70.00%> (ø)
js/classes/Calendar.js 68.64% <100.00%> (ø)
js/date-db-formatter.js 100.00% <100.00%> (ø)
js/user-preferences.js 92.45% <100.00%> (-0.14%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 82d51a0...0a09a37. Read the comment docs.

Copy link
Owner

@thamara thamara left a comment

Choose a reason for hiding this comment

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

Apart from my comment regarding the alignment of the content in the day view and the reuse of the API for formating the key, I think the PR is in pretty good shape.
I see a lot of code that is duplicated from the fixed calendar, but I think we most likely we'll replace completely the fixed one, so it should not be a problem.
Once this PR is done, I think we need to discuss a bit on the future of TTL w.r.t. the fixed vs flexible DBs.

js/classes/FlexibleDayCalendar.js Show resolved Hide resolved
js/classes/FlexibleMonthCalendar.js Outdated Show resolved Hide resolved
js/classes/FlexibleMonthCalendar.js Outdated Show resolved Hide resolved
js/classes/FlexibleDayCalendar.js Outdated Show resolved Hide resolved
js/classes/FlexibleDayCalendar.js Outdated Show resolved Hide resolved
js/classes/FlexibleDayCalendar.js Outdated Show resolved Hide resolved
js/classes/FlexibleMonthCalendar.js Outdated Show resolved Hide resolved
@araujoarthur0
Copy link
Collaborator Author

araujoarthur0 commented Oct 3, 2020

Yeah, I decided to copy styles and functions instead of reusing them from the other day calendar because we're still discussing that.
If we don't drop, I'm really going to advocate for a .scss styles organization, because we're over 1000 lines on our monolithic styles file 😋

@araujoarthur0
Copy link
Collaborator Author

Using generateKey, updated documentations to fit my other PR and alignment fix.

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.

Day View for Flexible Format
2 participants