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

[EZP-28683] Removes asterisk sign from labels when content editing #327

Merged
merged 2 commits into from
Jan 25, 2018
Merged

[EZP-28683] Removes asterisk sign from labels when content editing #327

merged 2 commits into from
Jan 25, 2018

Conversation

ViniTou
Copy link
Contributor

@ViniTou ViniTou commented Jan 24, 2018

Question Answer
Tickets https://jira.ez.no/browse/EZP-28676
Bug fix? yes
New feature? no
BC breaks? no
Tests pass? yes
Doc needed? no
License GPL-2.0

At first wanted to edit in _base-fields.scss but then it turns out that asterisk is added also in _forms.scss.
Dont know if it should be somehow excluded from .ez-data-source__label instead.

Checklist:

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

@@ -172,9 +172,15 @@
content: ': ';
}

&.required {
&.required:not(.ez-data-source__label) {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can avoid adding :not(.ez-data-source__label) here, as the next selector is a stronger than this one.

&:after {
content: '*: ';
}
}

&.required.ez-data-source__label {
Copy link
Member

Choose a reason for hiding this comment

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

Cannot we just drop using this mixin for .ez-data-source__label? https://github.com/ezsystems/ezplatform-admin-ui/blob/master/src/bundle/Resources/public/scss/fieldType/edit/_base-field.scss#L3

And just add &:after {content: ": "} https://github.com/ezsystems/ezplatform-admin-ui/blob/master/src/bundle/Resources/public/scss/fieldType/edit/_base-field.scss#L51 or in a twig?

Because for me it seems like generic mixin gets information about where is used and it's no longer generic. ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thought about this, but then it needs to be somehow duplicated in

I am right?

@barbaragr barbaragr self-assigned this Jan 25, 2018
@ViniTou
Copy link
Contributor Author

ViniTou commented Jan 25, 2018

@sunpietro
@dew326
I channeled inner frontend dev and tried to improve this. Can you take a look at that now? :)

@lserwatka lserwatka merged commit 720570e into ezsystems:1.0 Jan 25, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

5 participants