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

1307 content scheduling improvements #1349

Merged

Conversation

haringsrob
Copy link
Contributor

Description

In this pr:

  • We now use the date format defined in twill config for displaying the scheduled publication dates in the content tables
  • By default, under the same conditions as we allow to schedule we display the schedule start date (and end date if set)
  • Adds some label translations that were not yet translatable

There are some questions I left in code and also wondering if this is generally the correct approach.
@ifox could you have a look?

Related Issues

Fixes #1307

@haringsrob haringsrob requested a review from ifox January 5, 2022 09:41
@haringsrob
Copy link
Contributor Author

I made it so it is opt-in instead of default to have the column:

class ScheduledContentController extends BaseModuleController
{
    protected $moduleName = 'scheduledContents';

    protected $indexOptions = [
        'includeScheduledInList' => true,
    ];

@haringsrob
Copy link
Contributor Author

This should be ready now.

Copy link
Member

@ifox ifox left a comment

Choose a reason for hiding this comment

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

Thanks @haringsrob! Is the publish-on key used anywhere I missed?

@haringsrob
Copy link
Contributor Author

@ifox looks like it is not :D I will remove it

@haringsrob
Copy link
Contributor Author

@ifox how do you usually deal with lang.csv merge conflicts? 😅

@ifox
Copy link
Member

ifox commented Jan 14, 2022

I would rebase the branch ignoring it and then regen the CSV before force pushing.

@haringsrob
Copy link
Contributor Author

regen the CSV -> the command can only convert to the files from the csv or am I missing something?

@ifox
Copy link
Member

ifox commented Jan 14, 2022

There's a toCsv flag on the command :)

@haringsrob haringsrob force-pushed the 1307-content-scheduling-improvements branch from a3d1936 to c90a3f3 Compare January 14, 2022 13:31
@haringsrob
Copy link
Contributor Author

@ifox great will try that next time.

Did it manual for now so should be fine now

@@ -0,0 +1,17 @@
const state = {
publishDate24Hr: window[process.env.VUE_APP_NAME].STORE.config.publishDate24Hr,
Copy link
Member

Choose a reason for hiding this comment

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

It doesn't look like this isn't being used in Vue components, so we should be able to remove it.

@@ -82,6 +82,10 @@
window['{{ config('twill.js_namespace') }}'].twillLocalization = {!! json_encode($twillLocalization) !!};
window['{{ config('twill.js_namespace') }}'].STORE = {};
window['{{ config('twill.js_namespace') }}'].STORE.form = {};
window['{{ config('twill.js_namespace') }}'].STORE.config = {
publishDate24Hr: {{config('twill.publish_date_24h') ? 'true' : 'false'}},
Copy link
Member

Choose a reason for hiding this comment

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

It doesn't look like this isn't being used in Vue components, so we should be able to remove it.

@haringsrob
Copy link
Contributor Author

Hey @ifox conflict was resolved.

@haringsrob haringsrob merged commit a21d41a into area17:2.x Feb 25, 2022
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.

Custom language and date formatting
2 participants