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

IBX-792: Implemented Field Types Palette #1835

Merged
merged 25 commits into from
Sep 23, 2021
Merged

Conversation

adamwojs
Copy link
Member

@adamwojs adamwojs commented Aug 3, 2021

Question Answer
Tickets https://issues.ibexa.co/browse/IBX-792
Bug fix? no
New feature? yes
BC breaks? no
Tests pass? yes
Doc needed? ?
License GPL-2.0

REST endpoints for content type editing

TBD.

ibexa_field_group_name filter

ibexa_field_group_name filter allows to render human readable name of given field group. Usage

{{ field.group | ibexa_field_group_name }}

TODO:

  • Create endpoint adding field defintions to content type
  • Create endpoint removing field definition from content type
  • Create endpoint to reorder field definitions
  • Remove inlined CSS / JS
  • Implement Twig filter for field groups rendering
  • Clean up existing forms
  • Move to new namespaces

Checklist:

  • Coding standards ($ composer fix-cs)
  • Ready for Code Review

@adamwojs adamwojs self-assigned this Aug 6, 2021
@adamwojs adamwojs force-pushed the prototype_field_def_v2 branch 4 times, most recently from 3ce1f89 to f00c6bd Compare August 6, 2021 08:46
@adamwojs adamwojs force-pushed the prototype_field_def_v2 branch 3 times, most recently from 9128305 to 6f29170 Compare August 19, 2021 10:50
@tomaszszopinski
Copy link
Contributor

@adamwojs there seems to be a problem with notifications regarding CT creation; when a new CT is created (with its own custom name) successfully, then notification appears but with default CT name.

Zrzut ekranu 2021-08-24 o 14 33 10

@lucasOsti lucasOsti force-pushed the prototype_field_def_v2 branch 2 times, most recently from 1e238f7 to e3876bb Compare September 9, 2021 08:55
composer.json Outdated
"psr-4": { "EzSystems\\EzPlatformAdminUi\\Tests\\": "src/lib/Tests" }
"psr-4": {
"EzSystems\\EzPlatformAdminUi\\Tests\\": "src/lib/Tests",
"Ibexa\\AdminUi\\Tests\\": "src/lib/Tests"
Copy link
Member

Choose a reason for hiding this comment

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

This does not follow package structure requirements.

Suggested change
"Ibexa\\AdminUi\\Tests\\": "src/lib/Tests"
"Ibexa\\Tests\\AdminUi\\": "tests/lib"

Copy link
Contributor

@Steveb-p Steveb-p Sep 22, 2021

Choose a reason for hiding this comment

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

Fixed in ae4faf3.

Includes added tests directory to code style configuration & phpunit.xml adjustment.

EDIT: Also btw, the previous exclusion was not working at all :P

@@ -1,7 +1,7 @@
services:
EzSystems\EzPlatformAdminUiBundle\Controller\:
resource: "../../Controller/*"
exclude: "../../Controller/{Controller}"
exclude: "../../Controller/{FieldDefinitionController.php}"
Copy link
Member

Choose a reason for hiding this comment

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

TBH I don't like this workaround. I'd rather go for expanding this definition into specific services. It feels like explicitness is more clear and understandable.

This is remark rather for the future, for now let's leave as-is, since it was given as one of the options.

cc @Nattfarinn

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed, but the PR is big as it is, and from what I've seen I'd need to add most of the classes that reside in the subdirectory, which is around ~30 I believe?

@sonarcloud
Copy link

sonarcloud bot commented Sep 22, 2021

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.2% 0.2% Duplication

@Steveb-p
Copy link
Contributor

@ViniTou new namespaces have been introduced. You'd like to review the code again, or should I dismiss your review?

@dew326 dew326 requested a review from mnocon September 23, 2021 09:22
@dew326 dew326 merged commit b6817c4 into master Sep 23, 2021
@dew326 dew326 deleted the prototype_field_def_v2 branch September 23, 2021 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment