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-691: As a Developer, I want to have reusable popup menu component #1788

Merged
merged 3 commits into from
Jul 2, 2021

Conversation

tischsoic
Copy link
Contributor

@tischsoic tischsoic commented Jun 30, 2021

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

Popup menu component.

Checklist:

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

Base automatically changed from IBX-497-as-editor-i-want-to-see-redesigned-tabs to master June 30, 2021 10:07
@tischsoic tischsoic changed the title Popup menu component IBX-691: As a Developer, I want to have reusable popup menu component Jun 30, 2021
@tischsoic tischsoic removed the Feature label Jun 30, 2021
@tischsoic tischsoic marked this pull request as ready for review June 30, 2021 13:10
const popupMenuElement = tabsContainer.querySelector('.ibexa-popup-menu');

const popupMenu = new eZ.core.PopupMenu({
popupMenuElement: popupMenuElement,
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
popupMenuElement: popupMenuElement,
popupMenuElement,

const tabsContainer = popupMenuElement.parentElement;
const tabsList = tabsContainer.querySelector(SELECTOR_TABS_LIST);
const tabMore = tabsList.querySelector('.ibexa-tabs__tab--more');
const popupMenuAbsoluteLeft =
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
const popupMenuAbsoluteLeft =
const popupMenuLeftPosition =

?

'items': [],
}) }}
{% endif %}
{% if not hide_toggler|default(false) %}
<div class="ibexa-tabs__toggler">
<span class="ibexa-tabs__toggler-show">
{{ 'ui.component.tab.tabs.show'|trans|desc('Show') }}
{{ 'ui.component.tab.tabs_toggler.show'|trans|desc('Show') }}
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be extracted or such key exists?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I haven't noticed this change. Probably, by a mistake I have forgot to commit it in previous PR, because in xliff it has correct key: https://github.com/ezsystems/ezplatform-admin-ui/pull/1780/files#diff-fc54d07c9121de0ecf10e28736fa4c69f6e811c8f35b03d18f0a11ba1ef6a353R457

@@ -5,6 +5,12 @@
border-radius: $ibexa-border-radius;
box-shadow: calculateRem(4px) calculateRem(22px) calculateRem(67px) 0 rgba($ibexa-color-info, 0.2);
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we'll add this style to the variable?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Currently this shadow is used only here, so I think it should not be extracted as a global variable.

@sonarcloud
Copy link

sonarcloud bot commented Jul 1, 2021

Kudos, SonarCloud 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.0% 0.0% Duplication

@dew326 dew326 merged commit 9df22ac into master Jul 2, 2021
@dew326 dew326 deleted the popup-menu-component branch July 2, 2021 06:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants