Problem/Motivation

Some obsolete stuff according to Christian:

  • Move this hook function ui_patterns_field_config_delete(FieldConfigInterface $field_config): void TO ::UiPatternsEntitySchemaSubscriber ?
  • Remove test components alert, close_button, prop_types_tests ? >> Remove it and run phpunit to check
  • Remove ui-patterns-actions.html.twig >> Because hook theme was already removed

Some TODO following #3490872: [2.0.0-rc1] Remove temporary compatibilty layers:

$ grep -ir "todo" src/
src/Form/ComponentFormBuilderTrait.php:    // @todo This highlights the link between a configuration key and a form
src/SourcePluginBase.php:    // @todo select the shortest conversion path?
src/SourcePluginBase.php:   * @todo use NestedArray::mergeDeep ?
src/Element/ComponentFormBase.php:    // @todo better organize sources in groups.
src/Element/ComponentElementAlter.php:   * @todo Move this to Drupal Core.
src/Element/ComponentElementBuilder.php:    /* @todo Performance issue...
src/Plugin/UiPatterns/PropType/LinksPropType.php:        // @todo System path is deprecated - use the route name and parameters.
src/Plugin/UiPatterns/DerivableContext/EntityReferencedDerivableContext.php:    // @todo better implementation with service 'entity_type.bundle.info'
src/Plugin/UiPatterns/Source/TextfieldWidget.php:    // @todo change when issue https://www.drupal.org/project/drupal/issues/2633550 is fixed.
src/SourcePluginManager.php:    // @todo use a method of the plugin instead?

$ grep -ir "todo" modules/
modules/ui_patterns_views/src/ViewsPluginUiPatternsTrait.php:    // @todo better implementation with service 'entity_type.bundle.info'
modules/ui_patterns_field_formatters/src/Plugin/UiPatterns/Source/FieldFormatterSource.php:    // @todo remove ui patterns formatters from the list of options ?
modules/ui_patterns_field_formatters/src/Plugin/UiPatterns/Source/FieldFormatterSource.php:    // @todo Ensure it is right to empty all values here, see:
modules/ui_patterns_field_formatters/src/Plugin/Field/FieldFormatter/ComponentFormatterBase.php:    // @todo better implementation with service 'entity_type.bundle.info'
modules/ui_patterns_field_formatters/src/Plugin/Field/FieldFormatter/ComponentFormatterBase.php:    // @todo does this really makes sense to propagate the externally injected context

Which ones can you safely do before 2.0.0?

Proposed resolution

Remove what can be removed. Be careful.

Issue fork ui_patterns-3493134

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

pdureau created an issue. See original summary.

pdureau’s picture

pdureau commented
Assigned: Unassigned » christian.wiedemann
pdureau’s picture

pdureau commented
Assigned: christian.wiedemann » just_like_good_vibes

Mikael, can you have a look before Christian do it? Do you have other suggestions? Do you think some items from the list are risky?

pdureau’s picture

pdureau commented

Can we also remove this from ComponentElementBuilder::buildProp() ? 

    if (empty($data) && $prop_type->getPluginId() !== 'attributes') {
      // For JSON Schema validator, empty value is not the same as missing
      // value, and we want to prevent some of the prop types rules to be
      // applied on empty values: string pattern, string format, enum, number
      // min/max...
      // However, we don't remove empty attributes to avoid an error with
      // Drupal\Core\Template\TwigExtension::createAttribute() when themers
      // forget to use the default({}) filter.
      return $build;
    }

It looks like a security we may not need anymore

pdureau’s picture

pdureau commented
Assigned: just_like_good_vibes » christian.wiedemann

Mikael got a look, so back to Christian

pdureau’s picture

pdureau commented
Issue summary: View changes

christian.wiedemann made their first commit to this issue’s fork.

christian.wiedemann opened merge request !312

christian.wiedemann’s picture

christian.wiedemann commented

I removed everything I was sure it doesn' break anything. For that reason I did not remove:

if (empty($data) && $prop_type->getPluginId() !== 'attributes') {
      // For JSON Schema validator, empty value is not the same as missing
      // value, and we want to prevent some of the prop types rules to be
      // applied on empty values: string pattern, string format, enum, number
      // min/max...
      // However, we don't remove empty attributes to avoid an error with
      // Drupal\Core\Template\TwigExtension::createAttribute() when themers
      // forget to use the default({}) filter.
      return $build;
    }

Where is the logic moved? I think we need it.

And we need also ui_patterns_field_config_delete. We could optimize the code and call the event inside the hook but it is a very little optimization so I decided to let it as it is.

pdureau’s picture

pdureau commented

Let's create an other ticket for this specific empty($data) && $prop_type->getPluginId() !== 'attributes') issue.

When everything else is done (or skipped because considered unsafe) you can give the current ticket to Mikael for review

christian.wiedemann’s picture

christian.wiedemann commented
Assigned: christian.wiedemann » just_like_good_vibes
Status: Active » Needs review

just_like_good_vibes made their first commit to this issue’s fork.

just_like_good_vibes’s picture

just_like_good_vibes
Location PARIS
commented
Status: Needs review » Reviewed & tested by the community

looks ok for merge. then i just rebased to prepare merge

just_like_good_vibes’s picture

just_like_good_vibes
Location PARIS
commented
Assigned: just_like_good_vibes » Unassigned
Status: Reviewed & tested by the community » Fixed

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

christian.wiedemann’s picture

christian.wiedemann commented