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 decoupling from underscore => _.each pt2 => content overlays #8490

Merged

Conversation

nathanwoulfe
Copy link
Contributor

Prerequisites

  • I have added steps to test this contribution in the description below

Description

This is an addition to #8475, splitting into multiple PRs to avoid giant merges. These changes remove underscore from the content overlay controllers (the ones used for the publish/save/unpublish overlays). Also fixes the sort order of variants displayed in the overlays (were previously in reverse alphabetical order, now in alpha order).

Change replace all underscore methods with native JS, and makes some general improvements (avoids iterating collections multiple times where possible, syntax fixes etc). Functionally, nothing has changed.

Testing requirement is to verify everything still works. Can save, publish, save+publish, unpublish, schedule with variant and invariant content. Changes are pretty safe and predictable as most relate to iterating the variants collection, which is always an array even if content is invariant.

There's a lot of duplication across these controllers, as there's a lot of common functionality (sorting, filtering) which would be really good to refactor. Hypothetically (😃) Typescript would make this more manageable as a variant object could become a variant class with its own methods, so duplicated controller functions would no longer be required to check properties as the variant would provide those values...

export class Variant {
  // lots of variant properties
  language: VariantLanguage;
  segment: VariantSegment;
  state: string;

  get isMandatory(): boolean {
    return this.language && this.language.isMandatory && this.segment;
  }
}

// elsewhere
const mandatoryVariants = vm.variants.filter(v => v.isMandatory());

Oh so lovely!

@kjac kjac mentioned this pull request Jul 29, 2020
17 tasks
@nul800sebastiaan nul800sebastiaan merged commit ad1eda4 into umbraco:v8/contrib Jul 31, 2020
@nul800sebastiaan nul800sebastiaan added category/refactor community/pr release/8.8.0 release/no-notes This is too small to add to the release notes or fixed after a beta/RC type/feature and removed release/no-notes This is too small to add to the release notes or fixed after a beta/RC labels Jul 31, 2020
@nul800sebastiaan nul800sebastiaan changed the title 7731 bye bye underscore => _.each pt2 => content overlays Some decoupling from underscore => _.each pt2 => content overlays Jul 31, 2020
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.

3 participants