Skip to content

Conversation

@Timmy38
Copy link
Contributor

@Timmy38 Timmy38 commented Jan 19, 2026

No description provided.

@Timmy38 Timmy38 requested a review from steffunky January 19, 2026 09:54
@CombodoApplicationsAccount CombodoApplicationsAccount added the internal Work made by Combodo label Jan 19, 2026
width:450px;
}
.ibo-extension-details--information{
flex-grow:1;
Copy link
Member

Choose a reason for hiding this comment

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

There's some inconsistencies in your spacing,
The following format should be used:
rule-name:<space>rule-value;

justify-content: space-between;
align-items: center;

width:450px;
Copy link
Member

Choose a reason for hiding this comment

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

Put this in a variable,
also consider if this will be it's final width or if the parent grid will decide its element width

}
.ibo-extension-details--information--metadata{
@extend %ibo-font-ral-med-100;
color:$ibo-color-grey-700;
Copy link
Member

Choose a reason for hiding this comment

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

Put this in a variable

}

.ibo-extension-details--information--metadata span + span:before{
content: " - ";
Copy link
Member

Choose a reason for hiding this comment

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

Put this in a variable,
Also consider removing spaces and adding margin to each sides


.ibo-extension-details--actions > button{
position:relative;
top:-3px;
Copy link
Member

Choose a reason for hiding this comment

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

What's the point of the top: -3px ?
If we need to position flex element we can probably use justify-* or align-*

'force_uninstall',
'Force uninstall',
<<<JS
this.style.display = 'none';
Copy link
Member

Choose a reason for hiding this comment

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

This can probably be done with a complicated CSS rule, but it would be easier if you need to re-display this element if the input toggler is then re-toggled

This is not mandatory as of today but keep it in mind if you need this behavior

'Force uninstall',
<<<JS
this.style.display = 'none';
this.closest('.ibo-extension-details').querySelector('input[type=checkbox]').disabled = false
Copy link
Member

Choose a reason for hiding this comment

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

Do we leave the input disabled here ?

use Dict;
use utils;

class ExtensionDetailsFactory extends AbstractUIBlockFactory
Copy link
Member

Choose a reason for hiding this comment

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

UIBlocks factories tend to be named <BlockName>UIBlockFactory

$oBadgeInstalled = BadgeUIBlockFactory::MakeGreen('installed');
$oBadgeInstalled->AddCSSClass('checked');
$aBadges[] = $oBadgeInstalled;
$oBadgeToBeUninstalled = BadgeUIBlockFactory::MakeRed('to be uninstalled');
Copy link
Member

Choose a reason for hiding this comment

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

Missing dict entries, try to add them now before forgetting 😁
Check dictionaries/ui/layouts directory

@@ -0,0 +1 @@

Copy link
Member

Choose a reason for hiding this comment

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

If you don't plan to use this file for ExtensionDetail behavior you can delete it
Otherwise it can be kept

@Molkobain
Copy link
Contributor

@Timmy38 regarding the formatting of the SCSS, in PHPStorm hit Shift + Ctrl + Alt + L, then check Rearrange code. It will do it on the whole file.

@CombodoApplicationsAccount CombodoApplicationsAccount force-pushed the feature/8955_add_UIBlock_for_uninstallation branch from 46451a3 to 6dc8cef Compare January 21, 2026 14:35
@Molkobain
Copy link
Contributor

Discussed with Timothée IRL, we stay focused on the setup wizard for now, integration of the UIBlocks will be done afterwards.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

internal Work made by Combodo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants