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-1072: Redesign site factory #1921

Merged
merged 2 commits into from
Oct 12, 2021

Conversation

lucasOsti
Copy link
Contributor

@lucasOsti lucasOsti commented Sep 23, 2021

Question Answer
Tickets https://issues.ibexa.co/browse/IBX-1072
Related to https://github.com/ezsystems/ezplatform-site-factory/pull/142
Bug fix? no
New feature? yes
BC breaks? yes
Tests pass? yes
Doc needed? no
License GPL-2.0

Checklist:

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

@lucasOsti lucasOsti force-pushed the IBX-1072-redesign-site-factory branch 2 times, most recently from ec41e74 to 8451eb1 Compare September 28, 2021 09:46
@lucasOsti lucasOsti marked this pull request as draft September 29, 2021 12:18
@lucasOsti lucasOsti force-pushed the IBX-1072-redesign-site-factory branch from 69ea134 to 185ecf8 Compare October 1, 2021 08:28
@lucasOsti lucasOsti marked this pull request as ready for review October 1, 2021 08:28
@@ -16,6 +16,7 @@

&__header-label {
margin: 0;
font-size: calculateRem(16px);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
font-size: calculateRem(16px);
font-size: $ibexa-text-font-size;

}
}
}

&--heavy.ibexa-collapse--collapsed {
Copy link
Contributor

Choose a reason for hiding this comment

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

Move it to selector above

@@ -50,7 +50,7 @@

addItems(items, forceRecreate) {
if (this.isSingleSelect) {
this.inputField.value = items[0].id;
this.inputField.value = items[0]?.id || '';
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
this.inputField.value = items[0]?.id || '';
this.inputField.value = items[0]?.id ?? '';

?

@@ -1,5 +1,9 @@
{% import '@ezdesign/ui/component/macros.html.twig' as macros %}

<li class="ibexa-popup-menu__item {{ class|default('') }}">
<button type="button" class="ibexa-popup-menu__item-content {{ content_class|default('') }}" {{ macros.attributes(action_data_attr|default({})) }}>{{ label }}</button>
{% if action_data_attr.href is defined %}
Copy link
Contributor

Choose a reason for hiding this comment

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

href is not a data attribute.

line-height: calculateRem(24px);
}

&__bottom-info {
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe just __info? as I don't see other infos

@sonarcloud
Copy link

sonarcloud bot commented Oct 8, 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 1 Code Smell

No Coverage information No Coverage information
2.0% 2.0% Duplication

@dew326 dew326 merged commit ef980d4 into ezsystems:master Oct 12, 2021
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.

4 participants