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-29635: Styled labels in Admin Panel Edit views #632

Merged
merged 1 commit into from
Sep 28, 2018
Merged

EZP-29635: Styled labels in Admin Panel Edit views #632

merged 1 commit into from
Sep 28, 2018

Conversation

inakijv
Copy link
Contributor

@inakijv inakijv commented Sep 18, 2018

Question Answer
Tickets EZP-29635
Bug fix? yes
New feature? no
BC breaks? no
Tests pass? no
Doc needed? no
License GPL-2.0

Most important aspects:

  • Harmonized form labels in Editing views by styling existing class labels when permitted or by adding a new specific class ez-label, as well as adding a specific class for the view, ez-admin-edit;
  • Need help for exceptional cases:
    -> Content type specific Edit view for field type ezdatetime: Year, Month, Day, Minute, Second;
    -> Roles specific Edit view limitation group.

ez platform 1
ez platform 2
ez platform 4
ez platform 6
ez platform 7
ez platform 8
ez platform 10
ez platform 11
ez platform 12
ez platform 13
ez platform 14
ez platform 15

Would need help _>
-> Content type specific Edit view for field type ezdatetime: Year, Month, Day, Minute, Second;
-> Roles specific Edit view limitation group.

ez platform 3
ez platform 5

Checklist:

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

.ez-card {
.form-group,
.ez-card--fieldtype-container {
.ez-label {
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you think about making it a generic class. I mean not nested inside another classes.
Just something like this:

.ez-label {
    margin-bottom: 0;
    font-weight: 700;
    &:after {
        content: ': ';
    }
}

They same rule I would apply for the .required class.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would be great (I considered that option too). It will simplify more the process.

@inakijv inakijv changed the title Styled labels in Admin Panel Edit views EZP-29635: Styled labels in Admin Panel Edit views Sep 19, 2018
@inakijv
Copy link
Contributor Author

inakijv commented Sep 19, 2018

Updated based on review.

Still facing issues here:
-> Content type specific Edit view for field type ezdatetime: Year, Month, Day, Minute, Second;
-> Roles specific Edit view limitation group.

@sunpietro
Copy link
Contributor

@inakijv what are these issues you mentioned?

@inakijv
Copy link
Contributor Author

inakijv commented Sep 20, 2018

@sunpietro two views still need styling (I need suggestions/help on how to get them):

  • Admin panel / Content type Group / Content Type specific /Edit view (also Create view too) -> Field type ezdatetime, the labels: Year, Month, Day, Minute, Second;
  • Admin panel / Roles / Roles specific / Edit view -> labels for limitation group.

@@ -60,6 +60,28 @@
}
}

.ez-label {
Copy link
Member

Choose a reason for hiding this comment

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

If this is a separate component it should be in different file _label.scss.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok.

@inakijv
Copy link
Contributor Author

inakijv commented Sep 24, 2018

Updated based on review.

}
}

.ez-label.checkbox-inline,
Copy link
Contributor

Choose a reason for hiding this comment

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

It should go inside the .ez-label { declaration like this: &.checkbox-inline

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok.

}

.ez-label.checkbox-inline,
.ez-label.form-check-label {
Copy link
Contributor

Choose a reason for hiding this comment

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

It should go inside the .ez-label { declaration like this: &.form-check-label

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok.

}
}

.ez-label.form-check-label:not(.checkbox-inline) {
Copy link
Contributor

Choose a reason for hiding this comment

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

If you follow my suggestions above, then it should be placed inside the &.form-check-label { selector, like this: &:not(.checkbox-inline)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perfect. Thanks.

@inakijv
Copy link
Contributor Author

inakijv commented Sep 25, 2018

Updated based on review.

@barbaragr barbaragr self-assigned this Sep 26, 2018
Copy link

@barbaragr barbaragr left a comment

Choose a reason for hiding this comment

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

Editing URL looks different (different font, maybe we can change it for capital letters "URL"):
screen shot 2018-09-27 at 16 13 06

@inakijv
Copy link
Contributor Author

inakijv commented Sep 27, 2018

Updated based on review.

ez platform 27

@lserwatka lserwatka merged commit 003d6b0 into ezsystems:master Sep 28, 2018
ViniTou pushed a commit that referenced this pull request May 12, 2023
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.

6 participants