Search on subreddit and reddit feature with bug fixes #1
Conversation
…ugs with the loading window, added yarn as a dependency
It's stated that there isn't Linux support, but this is electron so it does, beautiful how easy it is right! You can fix this if you'd like unless you meant something else by not having Linux support listed.
|
Hi, |
savigny030
left a comment
savigny030
left a comment
There was a problem hiding this comment.
very nice features added. Thank u for ur effort! There are some little things that need to be changed.
greetings! :)
| let mainWindow; | ||
| let loadWindow; | ||
| //Created a variable to tell if the load page has already been displayed since interfered with the search functions, | ||
| let loaded = false; |
There was a problem hiding this comment.
ok so here we have a problem. This variable is a great idea from keeping the loading screen hidden, but there are two problems. first isnt that big. i thought it would be cool to show the loading screen when returning back from a maximised image hosted on reddit (i dunno if u kno but u can click on a image of e.g. a post and it will be shown in little viewer. if u click the cross in the top right there are sometimes a couple seconds delay untill the reddit thread page is shown again. but what do u think is it better without the loader? does it look annoying? the other thing is crutial. Mac apps dont close when pressing the window close cross. if u reopen the app then before the app was just restarted. but somehow now it only shows the loading screen. i dont know the exact reason but i can look into it further if u dont want to...
There was a problem hiding this comment.
to skip this problem u can use
// Quit when all windows are closed, except on macOS. There, it's common
// for applications and their menu bar to stay active until the user quits
// explicitly with Cmd + Q.
app.on('window-all-closed', () => {
if (process.platform !== 'darwin') {
app.quit()
}
})
app.on('activate', () => {
// On macOS it's common to re-create a window in the app when the
// dock icon is clicked and there are no other windows open.
if (BrowserWindow.getAllWindows().length === 0) {
loaded = false;
createMainWindow();
createLoadWindow();
}
})
i still have to check it but i think this will do the trick...
There was a problem hiding this comment.
ok so here we have a problem. This variable is a great idea from keeping the loading screen hidden, but there are two problems. first isnt that big. i thought it would be cool to show the loading screen when returning back from a maximised image hosted on reddit (i dunno if u kno but u can click on a image of e.g. a post and it will be shown in little viewer. if u click the cross in the top right there are sometimes a couple seconds delay untill the reddit thread page is shown again. but what do u think is it better without the loader? does it look annoying? the other thing is crutial. Mac apps dont close when pressing the window close cross. if u reopen the app then before the app was just restarted. but somehow now it only shows the loading screen. i dont know the exact reason but i can look into it further if u dont want to...
Ok I see, I originally added the loaded variable because the load screen had some conflicts with the search Reddit function. I actually do like the loading screen, the only slightly annoying thing about it is that it opens an entirely new window so if there is a way to show the load screen on the main window then that'd make it perfect.
There was a problem hiding this comment.
to skip this problem u can use
// Quit when all windows are closed, except on macOS. There, it's common // for applications and their menu bar to stay active until the user quits // explicitly with Cmd + Q. app.on('window-all-closed', () => { if (process.platform !== 'darwin') { app.quit() } }) app.on('activate', () => { // On macOS it's common to re-create a window in the app when the // dock icon is clicked and there are no other windows open. if (BrowserWindow.getAllWindows().length === 0) { loaded = false; createMainWindow(); createLoadWindow(); } })i still have to check it but i think this will do the trick...
I do not own a Mac so it would be impossible for me to test haha, so if you have one you can look and see if this might be a fix.
There was a problem hiding this comment.
ok so here we have a problem. This variable is a great idea from keeping the loading screen hidden, but there are two problems. first isnt that big. i thought it would be cool to show the loading screen when returning back from a maximised image hosted on reddit (i dunno if u kno but u can click on a image of e.g. a post and it will be shown in little viewer. if u click the cross in the top right there are sometimes a couple seconds delay untill the reddit thread page is shown again. but what do u think is it better without the loader? does it look annoying? the other thing is crutial. Mac apps dont close when pressing the window close cross. if u reopen the app then before the app was just restarted. but somehow now it only shows the loading screen. i dont know the exact reason but i can look into it further if u dont want to...
Ok I see, I originally added the loaded variable because the load screen had some conflicts with the search Reddit function. I actually do like the loading screen, the only slightly annoying thing about it is that it opens an entirely new window so if there is a way to show the load screen on the main window then that'd make it perfect.
Do you have slow internet or a slow machine? I though about it, too, but it wasnt notable on my main rig (ok obviously lol) and my old 2011 mac air... so I though it would be fine to open another window with the exact same dimensions...
There was a problem hiding this comment.
Do you have slow internet or a slow machine? I though about it, too, but it wasnt notable on my main rig (ok obviously lol) and my old 2011 mac air... so I though it would be fine to open another window with the exact same dimensions...
It's probably because I am running Ubuntu that it creates such a difference, it probably isn't like this for Window and Mac machines
There was a problem hiding this comment.
yes could be, I'm running fedora for work on my work laptop and on my main rig... I can check on that, too. If u wonder why i didnt release for linux in the first place, there is a huge bug (maybe was... it was in Feb. ) which didnt let me choose an Icon and i always had to rename the app in the package.json to meet som req. to build for linux. It was so horrible that I couldnt release. I will check for the new release if those bugs got addressed 😄
src/index.js
Outdated
| shell.openExternal(urlToGo).catch(err => { | ||
| if (err) console.log(err) | ||
| }) | ||
| } |
There was a problem hiding this comment.
yea still doesnt work somehow. if u click on a link in a post, like mine in my reddit post in e.g. r/electron the app opens the link in its own window, i dont know why yet...
There was a problem hiding this comment.
i think i commented the wrong line lol... I hope you know what i mean...
|
btw. when the changes are done, i will be implementing the auto update feature |
|
ok so to give you an update, i think i fixed the tab handling completely, also the mac fix did work and i changed your search feature to work with the loading screen. |
Ok awesome! UI on different operating systems can vary so it's great that you were able to test it. I am sure there is a way to make a loading screen on the same webpage using the 'preload' field in the 'webpreference' option when creating the main window. I think it should be fairly easy to implement; I could look into it today or later this week. |
Hi, fantastic, that would be awesome! I recently created a Issue regarding that and wrote down some Ideas, but yours sound great! #3 |
Hi Josh, did u made any progress? I'm sorry im writing all my exams at the time and I'm a bit slower in continuing the development. I don't want to overwrite your work if u have started on working on the loading spinner, but if u haven't i can look into it next week or sth... |
Oh man past two weeks have been really busy! Had zoom meetings every day prepping for this semester, sorry for the wait. I should be fairly free this week so I'll look into the on-page loading tomorrow! |
|
Yea, it seems like the workaround in commit ba9e86f created a bug in how images are displayed. I would recommend rolling back to the original fork and then make next release without the loading screen. In the meantime, we could work on better implementing the load screen! (Just to verify it's the best possible way). If not that's alright though. |
thanks for the feedback :) - coming to the bug, did u pulled the newest version of this branch? cause for me everything is displayed perfectly 😅 even using it via npm start... Or do you mean smth else then the maximised image window? |
Yea I pulled the newest, either way though I think having the onpage loading may fix the problem anyway, I've begun implementing it so hopefully it will. |
|
Alright so I found a better alternative to using the load screen, it's in this package and I think it would be very useful. I'm having trouble implementing it though, I've done it as a standalone but haven't been able to modify the codebase. Sorry I couldn't have been of more help! Here is an example to try and understand how it works. |
2 participants
Added the ability to search selected text on the subreddit currently viewing and all of Reddit. I also fixed a few bugs I experienced where the loading screen would pop up when clicking on intrasite links. Finally, I added yarn as a dependency since that was missing from the package.json and a small setup in the readme.