[WIP] Altered path mentioned in defaultIconUrl #589
Conversation
app/renderer/js/utils/domain-util.js
Outdated
| @@ -20,7 +20,7 @@ const logger = new Logger({ | |||
|
|
|||
| let instance = null; | |||
|
|
|||
| const defaultIconUrl = '../renderer/img/icon.png'; | |||
| const defaultIconUrl = path.resolve(__dirname, './../../img/icon.png'); | |||
There was a problem hiding this comment.
I'd remove the ./ here since it's redundant and looks clear without the ./.
| const defaultIconUrl = path.resolve(__dirname, './../../img/icon.png'); | |
| const defaultIconUrl = path.resolve(__dirname, '../../img/icon.png'); |
There was a problem hiding this comment.
Thank you for pointing that out (^_^)
Implemented the same in commit 46315ec
The icon previously was linked to the the path '../renderer/img/icon.png' which made it only possible for files having their working dirctory in the same directory as that of 'renderer' to access the file via the fs. Ideally one would like the path to be set such that the fallback image can be accessed effectively from any part of the repository. Using '__dirname' and 'path.resolve()', we get a universal path to the default icon so the fall back goes as intended.
|
@priyank-p I think this won't work since you can't access the file as it's packed in
|
|
I found this issue - Unable to access a file inside asar pacakge on Electron's Github page and this (comment) hints how it was solved. |
ghost
changed the title
|
@Flowkrad yeah, this might work. Can you try it out? |
|
Hope you are feeling better now (^_^)
Might be a while before I make a commit, have finals going on rn. If
someone else can implement it asap, then they can go ahead and do that but
it'll take a week for me before I can implement it.
Sorry for the dealay in getting back
…On Thu, Dec 6, 2018, 20:05 Akash Nimare ***@***.*** wrote:
@Flowkrad <https://github.com/flowkraD> yeah, this might work. Can you
try it out?
(sorry for the slow reply was sick)
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<https://github.com/zulip/zulip-electron/pull/589#issuecomment-444890903>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/An3DnXFIO78_7ls2MgCy4q02mchv6HQjks5u2SsVgaJpZM4X9daD>
.
|
|
@akashnimare Since this issue was unresolved, I had made PR (https://github.com/zulip/zulip-electron/pull/627) to fix the issue. Please review it. According to me, this PR (#589) won't be able to resolve the issue. |
|
Heads up @Flowkrad, we just merged some commits that conflict with the changes your made in this pull request! You can review this repository's recent commits to see where the conflicts occur. Please rebase your feature branch against the |
The fallback image path didn't lead back to the fallback image most likely due to the change in the folder structure. Using
__dirnameandpath.resolve(), we can now get a universal path to the given image.Issue in focus: #533
Previous PR made: #575 [Closed due to my inability to test on Windows, apologies for that]
You have tested this PR on: