1111829 - Add a dotted border as placeholders for missing favicons

Status: RESOLVED FIXED

(Content Services Graveyard :: Interest Dashboard, defect)

Description

[Marina Samuel [:emtwo]]

Attachment: UP-Cleo_Interface_STYLE_GUIDE_V1.pdf

The dotted border should look like the example on page 6, #6 under 'Most Visited Sites' in the attached document

Comment 1

[i.bharat.soni]

Marina, could you please assign this bug to me. I new to bugzilla, not sure how to assign a bug to me.

Comment 2

[Marina Samuel [:emtwo]]

Hi bharat,

You can assign a bug to yourself by putting in your email in the "Assigned To" field up top. I've assigned it to you now.

Comment 3

[i.bharat.soni]

Hi Marina,

I know this may sound dumb to you, could you please let me know where will be the code base for this and where to test if I fix this.

I have done all the installation part and currently running the project by running command : `./mach run`

Comment 4

[Marina Samuel [:emtwo]]

No worries! Valid question. The interest dashboard is an add-on and the code base lives here:
https://github.com/mozilla/interest-dashboard

You will need to install the add-on sdk to run/test it. Details here: https://developer.mozilla.org/en-US/Add-ons/SDK

You can find more detailed help on setup, testing, etc. Here:
https://wiki.mozilla.org/Content_Services/Interest_Dashboard/Get_Involved

Comment 5

[i.bharat.soni]

Fixed This one, Created PR:

https://github.com/iiison/interest-dashboard/commit/f1e8ebcb110017358d550f098559cadf4e649185

Comment 6

[i.bharat.soni]

Hi Marina,

Just wanted to know if you have reviewed/merged the PR I created, is the right way to merge the code.

Comment 7

[Ed Lee :Mardak]

i.bharat.soni, thanks for the pull request, next time it'll be easier to link directly to the pull request:

https://github.com/mozilla/interest-dashboard/pull/30

I've left some comments there about the code.

Also, do you have a good site that you tested with that shows the dashed border?

Comment 8

[i.bharat.soni]

Ed Lee, I've tested on my local system and it was working as expected, when there was no favicon, it showed dashed border.

Comment 9

[i.bharat.soni]

Ed Lee, I have updated the code, here is the link https://github.com/mozilla/interest-dashboard/pull/31,

Just wanted the know how to sync my forked branch from latest code.

Comment 10

[Ed Lee :Mardak]

(In reply to i.bharat.soni from comment #8)
> Ed Lee, I've tested on my local system and it was working as expected, when
> there was no favicon, it showed dashed border.
Oh, I was asking for a specific domain that you were able to see the dashed icon. I just couldn't think of a site that was being classified but didn't have an icon... although now that I think about it, I suppose the left column has an overall top sites even without classification.

(In reply to i.bharat.soni from comment #9)
> Just wanted the know how to sync my forked branch from latest code.
To be clear, mozilla:master has new code that isn't included in iiison:borderFix, but merging should work fine anyway. Having your changes on a dedicated branch is good practice for keeping separate work on different bugs/issues.

You can leave the borderFix branch as it is, but you might want to look into "git rebase" for the future.

Comment 11

[[github robot]]

Commit pushed to master at https://github.com/mozilla/interest-dashboard

https://github.com/mozilla/interest-dashboard/commit/306af2173c127d560f8782830a10c3cac6c66469
Merge pull request #31 from iiison/borderFix

Closes Bug 1111829 - Add a dotted border as placeholders for missing favicons