-
Notifications
You must be signed in to change notification settings - Fork 6
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
N°5554 Add refresh button for iTop 3.x #6
base: master
Are you sure you want to change the base?
Conversation
$oPage->AddSubBlock($oButtonUpCodeAndDb); | ||
} | ||
|
||
$oButtonSchema = ButtonUIBlockFactory::MakeForPrimaryAction("⟳ Refresh",null, null,false, "bt_content_schema"); | ||
$oButtonSchema->SetOnClickJsCode('CheckDBSchema(true);'); | ||
$oButtonSchema->SetIconClass("fas fa-redo"); |
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.
Tried to use this FontAwesome icon, but it doesn't seem to work. Any ideas?
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 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.
The FA stylesheet doesn't seem to be included in the page
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.
Oh.. Is that something that should be?
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.
To use the icons through CSS classes yes.
Looks like the font file is already included which might actually be a mistake actually. It should be the stylesheet not the font file. Font file will be brought by the stylesheet.
You can check how it is done in the backoffice and make a PR for the toolkit if you feel like it.
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.
Its just /css/font-awesome/css/all.min.css
right? Do you mean this url changes across versions?
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.
To me it would be ok to test for the existence of the file and include it. The toolkit is supposed to work with several versions of iTop as are some extensions, so it seems fair to me.
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.
Its just
/css/font-awesome/css/all.min.css
right? Do you mean this url changes across versions?
It could change yes
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.
But only as much as any other linked file from iTop core I guess?
Examples:
- ../images/indicator.gif
- js/jquery.ba-bbq.min.js
- ../images/itop-logo.png
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.
Yes, that's why we can consider it as an extension from my PoV. It relies on the iTop version installed and its resources, so it's ok to check if the file exists.
The "iTop update" tab didn't had a "Refresh" button anymore after initial DB schema check when on iTop > 3.
Only problem now is, for every refresh, a new button to update the DB is added:
![image](https://user-images.githubusercontent.com/228588/193043565-083c0058-ab8c-40ce-b184-9deb904fbe10.png)