-
Notifications
You must be signed in to change notification settings - Fork 109
Commit
Added Material icon. In the future, I want to continue to use them
- Loading branch information
There are no files selected for viewing
5 comments
on commit 59cc312
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.
Some issues with this commit:
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.
And how about more jquery-ish way?
$('<svg><use xlink:href="#ic_place_24px"></use></svg>')
.attr({
title:'Click to move to portal',
class:'material-icons icon-button',
})
.click(function() {
zoomToAndShowPortal(guid,[data.latE6/1E6,data.lngE6/1E6]);
if (isSmartphone()) {show('map')};
});
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.
Should I add material icon close, for further use?
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, I just suggest using .on('click' instead .click
As I can see in current code base there are a lot of both variants used here and there.
I prefer plain click
as it requires less parentheses, so all the code become slightly cleaner and easier to read.
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.
Should I add material icon close, for further use?
I am not sure yet.
We can do it in any moment, together with other changes of #45
SVG title doesn't show tooltip in this way.
We need here something like this:
But such tooltip does not get iitc style.
Perhaps we also need to improve this code:
ingress-intel-total-conversion/code/boot.js
Lines 460 to 474 in d77562b