-
Notifications
You must be signed in to change notification settings - Fork 279
N°8955 add UI block for uninstallation #797
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
base: feature/uninstallation
Are you sure you want to change the base?
N°8955 add UI block for uninstallation #797
Conversation
| width:450px; | ||
| } | ||
| .ibo-extension-details--information{ | ||
| flex-grow:1; |
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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: " - "; |
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
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'; |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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'); |
There was a problem hiding this comment.
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 @@ | |||
|
|
|||
There was a problem hiding this comment.
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
|
@Timmy38 regarding the formatting of the SCSS, in PHPStorm hit |
46451a3 to
6dc8cef
Compare
|
Discussed with Timothée IRL, we stay focused on the setup wizard for now, integration of the UIBlocks will be done afterwards. |
No description provided.