Skip to content

Conversation

@Julian1998
Copy link
Contributor

No description provided.

@coveralls
Copy link

coveralls commented Jun 14, 2018

Coverage Status

Coverage decreased (-0.9%) to 53.039% when pulling 056c4ba on cssAndTranslationUpdate into 16ab769 on master.

Copy link
Member

@DeepDiver1975 DeepDiver1975 left a comment

Choose a reason for hiding this comment

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

  • see comment
  • you wanted to remove the weekday picker in some cases - different PR?

<label class="pull-left inline">
<?php p($l->t('Repeat on every ...')); ?>
</label>
<div class="pull-right pull-half">
Copy link
Member

Choose a reason for hiding this comment

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

if I remove .pull-half it looks like this .... a bit better from my pov and consumes less or equal space

screenshot from 2018-06-14 21-01-12

@Julian1998 what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will update this PR in some days... ;)

<ul class="dropdown-menu" uib-dropdown-menu role="menu" aria-labelledby="single-button">
<li role="menuitem"><a ng-click="delete()" href="#"><?php p($l->t('Delete all')); ?></a></li>
<li role="menuitem"><a ng-click="deleteOccurrence()" href="#"><?php p($l->t('Delete just occurrence')); ?></a></li>
<li role="menuitem"><a ng-click="deleteOccurrence()" href="#"><?php p($l->t('Delete this event')); ?></a></li>
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we need to add the date and time of the explicit event.
After opening one event you basically get the series editor - the relation to 'this event' is missing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right! I would suggest that we "fix" the series editor. If you click on any repeating event you will just see the start and end time of the first event... So every occurrence should have it's own start and end time... But I think that's a way bigger problem because datetimepicker initialization and fullcalendar have no possibility to work together right now

-> new PR

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Julian1998
Copy link
Contributor Author

We have to limit recurrence rule combinations:

  • ByDay is not working in combination with monthly recurrence if interval is set to 1
  • A daily recurrence with byday option set is useless so remove byday for daily recurrences
  • It's the same for yearly recurrences -> Remove byday for yearly recurrences

@codecov-io
Copy link

codecov-io commented Aug 20, 2018

Codecov Report

Merging #922 into master will decrease coverage by 0.58%.
The diff coverage is 1.49%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #922      +/-   ##
==========================================
- Coverage   57.54%   56.95%   -0.59%     
==========================================
  Files          66       66              
  Lines        3295     3371      +76     
==========================================
+ Hits         1896     1920      +24     
- Misses       1399     1451      +52
Impacted Files Coverage Δ
js/app/models/simpleEventModel.js 88.47% <ø> (+0.51%) ⬆️
js/app/controllers/recurrencecontroller.js 1.03% <0%> (-0.7%) ⬇️
js/app/directives/datetimepickerDirective.js 8.88% <0%> (-0.21%) ⬇️
js/app/controllers/calcontroller.js 3.61% <5%> (-0.29%) ⬇️
controller/proxycontroller.php 100% <0%> (ø) ⬆️
controller/viewcontroller.php 96.26% <0%> (+0.26%) ⬆️
controller/contactcontroller.php 94.73% <0%> (+0.29%) ⬆️
controller/settingscontroller.php 78.29% <0%> (+0.32%) ⬆️

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 16ab769...706f98f. Read the comment docs.

Copy link
Member

@DeepDiver1975 DeepDiver1975 left a comment

Choose a reason for hiding this comment

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

  • more descriptive text in option dropdown
  • text alignment on repeat tab
  • custom tab: one line "Repeat every [int] [days|...]"
  • alignment of end
  • disable not selected end options
  • no weekdays on months
  • end options: never, after [int] events, at [datepicker]
  • no time on end date picker
  • on monthly: every [1|2|3|4|last] [monday|...]
  • delete button: "Delete this event (date)"

@DeepDiver1975
Copy link
Member

DeepDiver1975 commented Aug 23, 2018

  • we have to respect localization of the day. It is 23. in German but 23rd in English - maybe mometjs can help?

@DeepDiver1975
Copy link
Member

see https://momentjs.com/docs/#/displaying/format/ - use format string Do

@DeepDiver1975
Copy link
Member

DeepDiver1975 commented Aug 23, 2018

  • left alligning the two inputs fields on the end section would be great - like google calendar does ...

@DeepDiver1975 DeepDiver1975 force-pushed the cssAndTranslationUpdate branch from be2e543 to 056c4ba Compare September 2, 2018 10:02
@DeepDiver1975 DeepDiver1975 force-pushed the cssAndTranslationUpdate branch from 056c4ba to 706f98f Compare September 2, 2018 10:15
@DeepDiver1975 DeepDiver1975 merged commit db3c88a into master Sep 2, 2018
@DeepDiver1975 DeepDiver1975 deleted the cssAndTranslationUpdate branch September 2, 2018 10:16
@DeepDiver1975 DeepDiver1975 mentioned this pull request Sep 19, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants