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

Some Simple calendar improvements #234

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

sarjona
Copy link
Contributor

@sarjona sarjona commented Nov 24, 2016

First of all, thanks for your plugin. We're using it in some of our projects :-)

We've fixed some bugs and included some improvements. We would like to share with you them:

  • By default, exclude grouped calendars from get_calendars function and Remove taxonomies associated to grouped calendar for preventing errors
    This 2 patches are necessary to avoid deadlock when users add a grouped calendar to a category or when users add a grouped calendar to another grouped calendar (which contains the first one).

  • Load localization file
    Without this patch, we're not able to load the catalan .po file.

  • Show event information on mouse over
    It's an improvement to simulate a behaviour in the Google calendar events legacy version. With this patch, events information is showed on mouse over (it's not necessary to click).

@nickyoung87
Copy link
Contributor

Thank you for your time and submitting a PR. I haven't looked over everything yet, but something did stick out to me so I want to ask about it. The hover feature you mentioned - is that not the same as this setting?

http://www.screencast.com/t/9Kfjy3Sg2Nm

@pderksen
Copy link
Contributor

pderksen commented Dec 9, 2016

@sarjona Also thanks for submitting.

I'll need some time to review the grouped calendar/taxonomies change. Maybe an example screencast and/or screenshots would help display your scenario.

For the localization file, load_plugin_textdomain is in fact called with the plugins_loaded action hook, it just resides in a different function (in turn called by the class constructor).

https://github.com/moonstonemedia/Simple-Calendar/blob/4440624fdee6151290d2cc759e6ad83384955458/includes/main.php#L188-L211

@sarjona
Copy link
Contributor Author

sarjona commented Dec 29, 2016

@nickyoung87 This hover feature only works in the page where the full calendar is showed (like http://agora.xtec.cat/formacio/primaria/calendar/145/). Instead, the patch works always with the widget (because in this case we've found that users prefer hover in all the cases). You can see an example in the following screenshot: http://www.screencast.com/t/7e7CXGrh

@sarjona
Copy link
Contributor Author

sarjona commented Dec 29, 2016

@pderksen You're wellcome!! :-)

  • Without the "non-deadlock" patches:
  1. The user can create 2 different grouped calendars and call one from the other and viceversa, like you can see in the following screenshots: http://www.screencast.com/t/bdISH2qhJ and http://www.screencast.com/t/mQPugdOLQD Our patch exclude grouped calendars of the manual selection list.
  2. With the categories happens something similar: you can add some category to a grouped calendar (for instance Category1), and then configure the grouped calendar to load: http://www.screencast.com/t/g9qIkui3 Our patch don't let to select categories for grouped calendars.

@pderksen
Copy link
Contributor

pderksen commented Jan 5, 2017

@sarjona Sorry for the delay. We're still backlogged with projects. We're hoping to address this next week sometime at this point.

@roxaubrey
Copy link

Using this plugin because it's GREAT, however, when I hover over an event, the window shows up super low on my screen and is cut off. I apologize for being a noob, but is there an adjustment I can make in the CSS? I've tried everything I know (which isn't much) and nothing has worked.

@nickyoung87
Copy link
Contributor

@roxaubrey We already have that issue documented at #221 Please post on that issue and not this pull request.

@cliffordp
Copy link

cliffordp commented Dec 28, 2017

load_plugin_textdomain is not needed if you let translate.wordpress.org do the translations and you change the readme.txt header's Requires at least: to 4.6, per https://developer.wordpress.org/plugins/internationalization/how-to-internationalize-your-plugin/#loading-text-domain

https://make.wordpress.org/meta/handbook/documentation/translations/ may also be helpful to know that you can import your plugin's existing/legacy translations.

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.

6 participants