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-6398: Adjust admin-ui for Image Picker component #956

Merged
merged 3 commits into from
Nov 20, 2023

Conversation

tischsoic
Copy link
Contributor

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

Checklist:

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

'adminUiConfig.universalDiscoveryWidget.tabs',
[
{
id: 'image_picker',
Copy link
Contributor

Choose a reason for hiding this comment

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

adding the same tab with a different id? could you explain why?
additionally: admin-ui should not have any knowledge about image-picker

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 removed it. The backend part for overwriting config will be done in a separate PR.

Copy link
Contributor

@GrabowskiM GrabowskiM left a comment

Choose a reason for hiding this comment

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

Except Darek's comment (and possibly mine about memo)

@@ -137,7 +137,23 @@ export const SearchTextContext = createContext();
export const DropdownPortalRefContext = createContext();

const UniversalDiscoveryModule = (props) => {
const { tabs } = ibexa.adminUiConfig.universalDiscoveryWidget;
const { tabs: tabsWithPriority } = ibexa.adminUiConfig.universalDiscoveryWidget;
const tabs = tabsWithPriority.reduce((tabsPrioritized, tabToAdd) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe put it in memo? Not sure if it's easy as you have to pass array

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 removed this approach.

@tischsoic tischsoic force-pushed the IBX-6398-image-picker-base branch from 5af47c7 to d49ad07 Compare November 17, 2023 14:49
Copy link

sonarcloud bot commented Nov 17, 2023

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
0.0% 0.0% Duplication

@micszo micszo removed their assignment Nov 20, 2023
@dew326 dew326 merged commit 24a1ecc into main Nov 20, 2023
23 checks passed
@dew326 dew326 deleted the IBX-6398-image-picker-base branch November 20, 2023 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants