Working on #2972 Template Conversion to Twig Format (calendar.php) #2984

Starpaul20 · 2018-01-12T05:51:55Z

Working on #2972

This PR is for the calendar (calendar.php).

Part 1 - Adding an event

Part 2 - Editing an event

Part 3 - Moving event

Part 4 - Viewing an event

euantorano

This is a big one! Great work so far 😄

euantorano · 2018-01-12T23:22:15Z

calendar.php

+        $years[] = $year;
+    }
+
+    $timezones = build_timezone_select("timezone", $timezone);


I wonder if this is worth making a Twig function - do we use the same function elsewhere (such as the User CP)?

As far as I'm aware it's the same everywhere. So it could be a Twig function

Alright, sounds good. Not sure whether we should have a TimeDateExtension for this and my_date (and whether we have any other time/date related functions to add to it) or if we should just create a CoreExtension/MybbExtension with these kinds of functions...

euantorano · 2018-01-12T23:23:10Z

calendar.php

+    // Construct option list for years
+    for ($year = my_date('Y'); $year < (my_date('Y') + 5); ++$year) {
+        if ($year == $single_year) {
+            $select['single_year'] = "selected=\"selected\"";


Would be nice to do this logic inside the view rather than building HTML in the backend code. Same goes with the other instances of this type of code.

I agree, but I'm not sure how to do that.

Two options:

Do the conditional check in Twig

Set a variable such as $select['single_year_selected'] = true and in twig do something like:

<option{%if select.single_year_selected %} selected="selected"{%endif}/>

Option two is the easiest by far.

euantorano · 2018-01-12T23:25:44Z

inc/views/base/calendar/addevent.twig

+            <tr>
+                <td class="trow2" style="vertical-align: top;"><strong>{{ lang.event_date }}</strong></td>
+                <td class="trow1">
+                    <script type="text/javascript">


A note for @justinsoltesz @Eric-Jackson - would be nice to take this inline JS out. The less inline JS we have, the better in my opinion.

Would be great if we could have none so that admins can use a CSP without unsafe-inline.

Yeah same goes for inline CSS imo, which is happening after the conversion to Twig I believe. Not sure if it would be better to remove JS now, or wait until we're making the changes to the HTML for a responsive theme.

euantorano · 2018-01-12T23:26:53Z

inc/views/base/calendar/addevent.twig

+                    <dl style="margin-top: 0; margin-bottom: 0; width: 100%;">
+                        <dt><input type="radio" name="type" value="single" {{ select.type_single }} id="type_single" onclick="toggleType();" /> <label for="type_single"><strong>{{ lang.type_single }}</strong></label></dt>
+                        <dd style="margin-top: 4px;" id="single_selects">
+                            <select name="single_day">


Perhaps a good idea to try and abstract this into a loop and possibly break it into a function or partial view or something?

Part 5 - Day view

Part 6 - Week view

Part 7 - Calendar

Starpaul20 · 2018-01-13T23:47:29Z

I've finished the conversion work on the calendar. Once I fix the above issues (I hope to do them tomorrow) this will be ready to merge.

One thing I want to ask everyone, should we keep the $gobutton function? I've been including it so far as {{ gobutton|raw }} but it seems pointless. All it does is load a template that contains a submit button, something that can be done inline easily.

euantorano · 2018-01-14T07:20:19Z

I just moved it inline for the subscriptions template personally.

…

On 13 Jan 2018, at 23:47, Paul Bender ***@***.***> wrote: I've finished the conversion work on the calendar. Once I fix the above issues (I hope to do them tomorrow) this will be ready to merge. One thing I want to ask everyone, should we keep the $gobutton function? I've been including it so far as {{ gobutton|raw }} but it seems pointless. All it does is load a template that contains a submit button, something that can be done inline easily. — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

-Added selected and checked HTML to templates -Move day select options to array

Starpaul20 · 2018-01-14T17:17:05Z

That should be everything. If there are no other issues this can be merged.

euantorano

Justa few more minor comments. This is a big bunch of templates, thanks a lot for working on it.

Note that I'm not sure the loop idea will work at all, but it's worth testing.

euantorano · 2018-01-14T20:17:51Z

inc/views/base/calendar/addevent.twig

+                            </select>
+                            &nbsp;
+                            <select name="single_month">
+                                <option value="1"{% if select.single_month.1 %} selected="selected"{% endif %}>{{ lang.month_1 }}</option>


Not a big deal, but repeated blocks like these where it's easy to forget to update a single number make me anxious. The problem with using a loop here would be the month language variable, right? If so, you an kind of cheat:

{% for i in 1..12 %} <option value="{{ i }}"{% if select.single_month[i] %} selected="selected"{% endif %}>{{ attribute(lang, 'month_' ~ i) }}</option> {% endfor %}

The trick here is using the [] square brackets for the condition, and the attribute core Twig function to get the language variable (with string concatenation using the ~ operator).

I believe that should work, but I haven't tested it at all.

euantorano · 2018-01-14T20:18:35Z

inc/views/base/calendar/calendar.twig

+                        {% if calendar.prev_link %}
+                            <a href="{{ calendar.prev_link|raw }}">&laquo; {{ calendar.prev_month_name }} {{ calendar.prev_month_year }}</a>
+                        {% endif %}
+                        {% if calendar.sep == true %}


Could simplify this to {% if calendar.sep %}, since you already use the short form for next_link below (unless this can be a string or some other type or something...).

euantorano · 2018-01-14T20:19:34Z

inc/views/base/calendar/calendar.twig

+                            <select name="month">
+                                <option value="{{ calendar.month }}">{{ calendar.currentmonth }}</option>
+                                <option value="{{ calendar.month }}">----------</option>
+                                <option value="1">{{ lang.alt_month_1 }}</option>


Again, could use the same loop trick I mentioned in the other template file hopefully.

euantorano · 2018-01-14T20:19:53Z

inc/views/base/calendar/dayview.twig

+                            <select name="month">
+                                <option value="{{ calendar.month }}">{{ calendar.currentmonth }}</option>
+                                <option value="{{ calendar.month }}">----------</option>
+                                <option value="1">{{ lang.alt_month_1 }}</option>


Loop trick again 😉

euantorano · 2018-01-14T20:20:23Z

inc/views/base/calendar/weekview.twig

+                            <select name="month">
+                                <option value="{{ calendar.week_from_month }}">{{ calendar.week_from_one }}</option>
+                                <option value="{{ calendar.week_from_month }}">----------</option>
+                                <option value="1">{{ lang.alt_month_1 }}</option>


Loop again 😄

- Moved month select options to array - Split bottom calendar jump/month jump into seperate Twig file - Converted user stars to Twig

Starpaul20 · 2018-01-14T21:42:07Z

I've converted the most months to use for loops. The ones I didn't I moved to their own file and included it where necessary (so there's only one instance instead of four).

euantorano · 2018-01-14T22:10:50Z

Nice one, thanks. Looks much cleaner now!

euantorano · 2018-01-14T22:13:24Z

@PaulBender Can you update the template list in #2972 with the templates you've sorted with this?

…2984) * Working on #2972 Template Conversion to Twig Format (calendar.php) Part 1 - Adding an event * Working on #2972 Template Conversion to Twig Format (calendar.php) Part 2 - Editing an event * Working on #2972 Template Conversion to Twig Format (calendar.php) Part 3 - Moving event * Working on #2972 Template Conversion to Twig Format (calendar.php) Part 4 - Viewing an event * Working on #2972 Template Conversion to Twig Format (calendar.php) Part 5 - Day view * Working on #2972 Template Conversion to Twig Format (calendar.php) Part 6 - Week view * Working on #2972 Template Conversion to Twig Format (calendar.php) Part 7 - Calendar * Moved gobutton to inline * Code updates -Added selected and checked HTML to templates -Move day select options to array * More code updates - Moved month select options to array - Split bottom calendar jump/month jump into seperate Twig file - Converted user stars to Twig

mybb#2984) * Working on mybb#2972 Template Conversion to Twig Format (calendar.php) Part 1 - Adding an event * Working on mybb#2972 Template Conversion to Twig Format (calendar.php) Part 2 - Editing an event * Working on mybb#2972 Template Conversion to Twig Format (calendar.php) Part 3 - Moving event * Working on mybb#2972 Template Conversion to Twig Format (calendar.php) Part 4 - Viewing an event * Working on mybb#2972 Template Conversion to Twig Format (calendar.php) Part 5 - Day view * Working on mybb#2972 Template Conversion to Twig Format (calendar.php) Part 6 - Week view * Working on mybb#2972 Template Conversion to Twig Format (calendar.php) Part 7 - Calendar * Moved gobutton to inline * Code updates -Added selected and checked HTML to templates -Move day select options to array * More code updates - Moved month select options to array - Split bottom calendar jump/month jump into seperate Twig file - Converted user stars to Twig

…2984) * Working on #2972 Template Conversion to Twig Format (calendar.php) Part 1 - Adding an event * Working on #2972 Template Conversion to Twig Format (calendar.php) Part 2 - Editing an event * Working on #2972 Template Conversion to Twig Format (calendar.php) Part 3 - Moving event * Working on #2972 Template Conversion to Twig Format (calendar.php) Part 4 - Viewing an event * Working on #2972 Template Conversion to Twig Format (calendar.php) Part 5 - Day view * Working on #2972 Template Conversion to Twig Format (calendar.php) Part 6 - Week view * Working on #2972 Template Conversion to Twig Format (calendar.php) Part 7 - Calendar * Moved gobutton to inline * Code updates -Added selected and checked HTML to templates -Move day select options to array * More code updates - Moved month select options to array - Split bottom calendar jump/month jump into seperate Twig file - Converted user stars to Twig

Starpaul20 added 4 commits January 12, 2018 00:49

Working on #2972 Template Conversion to Twig Format (calendar.php)

41a15bc

Part 1 - Adding an event

Working on #2972 Template Conversion to Twig Format (calendar.php)

c38dfaa

Part 2 - Editing an event

Working on #2972 Template Conversion to Twig Format (calendar.php)

1dfdc94

Part 3 - Moving event

Working on #2972 Template Conversion to Twig Format (calendar.php)

892e810

Part 4 - Viewing an event

euantorano requested changes Jan 12, 2018

View reviewed changes

Working on #2972 Template Conversion to Twig Format (calendar.php)

6d32738

Part 5 - Day view

Ben-MyBB added the b:1.9 Branch: 1.9.x label Jan 13, 2018

Starpaul20 added 2 commits January 13, 2018 14:05

Working on #2972 Template Conversion to Twig Format (calendar.php)

d091f65

Part 6 - Week view

Working on #2972 Template Conversion to Twig Format (calendar.php)

8703cad

Part 7 - Calendar

Starpaul20 added 2 commits January 14, 2018 09:51

Moved gobutton to inline

964a8dc

Code updates

0a83dbd

-Added selected and checked HTML to templates -Move day select options to array

euantorano added t:enhancement Type: Enhancement. Contains minor improvements s:in-progress Status: In Progress. Some work completed s:resolved Status: Resolved. Solution implemented or scheduled and removed s:in-progress Status: In Progress. Some work completed labels Jan 14, 2018

euantorano requested changes Jan 14, 2018

View reviewed changes

More code updates

9ae62e1

- Moved month select options to array - Split bottom calendar jump/month jump into seperate Twig file - Converted user stars to Twig

euantorano merged commit d398853 into mybb:develop/1.9 Jan 14, 2018

Starpaul20 deleted the develop-1.9-calendar branch January 14, 2018 22:23

euantorano mentioned this pull request Jan 4, 2020

Develop/1.9 #3857

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Working on #2972 Template Conversion to Twig Format (calendar.php) #2984

Working on #2972 Template Conversion to Twig Format (calendar.php) #2984

Starpaul20 commented Jan 12, 2018 •

edited

euantorano left a comment

euantorano Jan 12, 2018

Starpaul20 Jan 13, 2018

euantorano Jan 13, 2018

euantorano Jan 12, 2018

Starpaul20 Jan 13, 2018

euantorano Jan 13, 2018

euantorano Jan 12, 2018

JordanMussi Jan 13, 2018

Eric-Jackson Jan 23, 2018

euantorano Jan 12, 2018

Starpaul20 commented Jan 13, 2018

euantorano commented Jan 14, 2018 via email

Starpaul20 commented Jan 14, 2018

euantorano left a comment

euantorano Jan 14, 2018

euantorano Jan 14, 2018

euantorano Jan 14, 2018

euantorano Jan 14, 2018

euantorano Jan 14, 2018

Starpaul20 commented Jan 14, 2018

euantorano commented Jan 14, 2018

euantorano commented Jan 14, 2018

Working on #2972 Template Conversion to Twig Format (calendar.php) #2984

Working on #2972 Template Conversion to Twig Format (calendar.php) #2984

Conversation

Starpaul20 commented Jan 12, 2018 • edited

euantorano left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Starpaul20 commented Jan 13, 2018

euantorano commented Jan 14, 2018 via email

Starpaul20 commented Jan 14, 2018

euantorano left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Starpaul20 commented Jan 14, 2018

euantorano commented Jan 14, 2018

euantorano commented Jan 14, 2018

Starpaul20 commented Jan 12, 2018 •

edited