Skip to content
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

Feature/bukuserver readme update #657

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

rachmadaniHaryono
Copy link
Collaborator

@rachmadaniHaryono rachmadaniHaryono commented Jan 6, 2023

related

task

  • list all the changes made in the bukuserver side since the last release

description

  • imgur album is updated
    • add version
    • add link to buku repo
  • added screenshot with new theme
  • tag and link on album

e: for completion

screenshot is taken on 1920x768 with 110% zoom on firefox

links on screenshot

https://github.com/softashell/iqdb_tagger
https://aaroncake.net/
https://www.allaboutcircuits.com/
https://www.alldatasheet.com/
https://starthardware.org/

e2: link to share on new release https://i.imgur.com/KaZIezk.png

bukuserver/README.md Outdated Show resolved Hide resolved
@LeXofLeviafan
Copy link
Collaborator

As mentioned here, the new screenshots are still disproportional due to including empty side margins (which is particularly visible when viewing the gallery on Imgur).

Also note the suggestion to add a screenshot with a hyperlink (e.g. pointing to the screenshots section of bukuserver readme) into the main readme, for sake of feature visibility. (The bookmarks list should do – maybe even the dark one.)

…Another suggestion from the same comment – add tags to the gallery, and update image order and titles to match the readme. (Also not sure that keeping in the gallery a screenshot which doesn't appear in the readme is a good idea.)


P.S. @jarun your opinion on enabling favicons by default (for sake of user-friendliness)? I've mentioned it there as well but it doesn't seem like anyone noticed it 😅

@jarun
Copy link
Owner

jarun commented Jan 6, 2023

I am OK with enabling the favicons by default.

@rachmadaniHaryono
Copy link
Collaborator Author

i am against it so it is 1-2 on me.

but i will change it but add info why you should / should not enable favicon

i will add more commit today for some suggestion i missed from LeXofLeviafan

@jarun
Copy link
Owner

jarun commented Jan 7, 2023

@rachmadaniHaryono why so? I may have missed. Can you please linked to your arguments?
My intention was to make it more attractive to users.

@rachmadaniHaryono
Copy link
Collaborator Author

it is disabled on #503 reason stated here #484 (comment)

i think there was user who dont want to ping to google when using bukuserver but i cant find that

@jarun
Copy link
Owner

jarun commented Jan 7, 2023

Got it! It's a valid concern. Let's not do it.
Another problem with networked operations is - they have the potential to slow things down in case of a bad network and deliver a horrible user experience.

@jarun
Copy link
Owner

jarun commented Jan 7, 2023

We don't want any non-user triggered network activity. Period.

@LeXofLeviafan
Copy link
Collaborator

Same here – the UI looks better and more "full-featured" with favicons; moreover, the users will see favicons on the screenshots, and will thus expect to see them the first time they run bukuserver with no extra fiddling (otherwise they'll think of it as false advertisement… and no, I'm pretty sure that if favicons are removed from the screenshots, most users would simply be unaware of the feature – not to mention that the screenshot with favicons look indeed more visually attractive than "text-only" one).

…Speaking of things missing on the screenshots: I just realised that even though bookmarks list is visible in most screenshots of the gallery, only one of them has bookmark descriptions on it (and ironically, it's not complete either, cuz it has no tags).

Moreover, only dark-themed screenshots have netlocs in them (which is actually kinda impressive cuz it shouldn't be possible without changing URL render mode to netloc – and that's clearly not what's going on here). Or are these screenshots that old? Netlocs seem to be present in v4.7, at least.

@rachmadaniHaryono
Copy link
Collaborator Author

Another problem with networked operations is - they have the potential to slow things down in case of a bad network and deliver a horrible user experience.

actually not that bad. if network not working it will be just empty

@rachmadaniHaryono
Copy link
Collaborator Author

Moreover, only dark-themed screenshots have netlocs in them (which is actually kinda impressive cuz it shouldn't be possible without changing URL render mode to netloc – and that's clearly not what's going on here). Or are these screenshots that old? Netlocs seem to be present in v4.7, at least.

that very first screenshots before bukuserver use flask-admin

@LeXofLeviafan
Copy link
Collaborator

that very first screenshots before bukuserver use flask-admin

WDYM?

@rachmadaniHaryono
Copy link
Collaborator Author

rachmadaniHaryono commented Jan 7, 2023

i mean it is created while i develop the very first version of bukuserver with flask-admin


but after i check git history there are older screenshots

for example https://i.imgur.com/tnRWsds.png

https://github.com/jarun/buku/blob/fb843225eed1f8dca5982485afb01b8b946eb94c/bukuserver/README.md

@LeXofLeviafan
Copy link
Collaborator

LeXofLeviafan commented Jan 7, 2023

The image description says "Bookmark entries with favicon v4.7" etc. though 🤔

…Either way, I say it's high time to update the screenshots.

P.S. If we won't be enabling favicons by default, I suggest clarifying they have to be enabled and/or linking directly to the respective part of the docs in the screenshot description
(i.e. _bookmark page [with favicon enabled](#configuration)_)

@rachmadaniHaryono
Copy link
Collaborator Author

rachmadaniHaryono commented Jan 7, 2023

The image description says "Bookmark entries with favicon v4.7" etc.

for more exact text it should be "v<=4.7" but i decided to put v4.7

there maybe exact version but at least on v4.7 you got this with some configuration

this also why i dont put version on new screenshot with slate theme

little bit related to that, what is next version @jarun? 5.0 or 4.8 or something else?

P.S. If we won't be enabling favicons by default, I suggest clarifying they have to be enabled and/or linking directly to the respective part of the docs in the screenshot description
(i.e. bookmark page with favicon enabled)

agree with this

e:

new screenshot

maybe not all. i have to check it

for example we can kept statistic page because it is not changed at all

you also mention body width side-margins, i will look if imgur can edit it online

@LeXofLeviafan
Copy link
Collaborator

I reckon the simplest way to deal with this would be to make a new gallery (so that the old screenshots are still accessible); I also somehow doubt that Imgur supports replacing images without changing permalinks so "editing online" is probably out of the question anyway.

…Although I'm not sure if they allow having the same image (with identical permalinks) in multiple galleries. It might be a possibility tho, seeing as "individual image pages" do not link back to their original galleries.

for more exact text it should be "v<=4.7" but i decided to put v4.7

I believe the correct way of naming them would be to use the version for which these screenshots were added (according to git blame, the new screenshots were committed after release of v3.8, around the same time when netlocs were added). I suggest placing the version in the name of the old gallery instead, though.


P.S. I just realised there's a small issue with the site header: it contains a quick search field on Home & Statistic pages, but not on Bookmarks & Tags. Seeing as it's the site header, I believe it should not differ much depending on the page.

…There also might be a minor bug related to the auto-closing functionality; I'll try investigating it before the release.

bukuserver/README.md Show resolved Hide resolved
bukuserver/README.md Outdated Show resolved Hide resolved
@LeXofLeviafan
Copy link
Collaborator

…There also might be a minor bug related to the auto-closing functionality; I'll try investigating it before the release.

…Opened an issue and made a pull request for the bug.

@rachmadaniHaryono
Copy link
Collaborator Author

I reckon the simplest way to deal with this would be to make a new gallery (so that the old screenshots are still accessible); I also somehow doubt that Imgur supports replacing images without changing permalinks so "editing online" is probably out of the question anyway.

i tried imgur online editor and it is not enough so as suggested i will make new gallery

i will post picture here first and then upload it after agreed

README.md Outdated Show resolved Hide resolved
@jarun
Copy link
Owner

jarun commented Jan 10, 2023

@LeXofLeviafan please let me know when this is done.

@LeXofLeviafan
Copy link
Collaborator

@LeXofLeviafan please let me know when this is done.

@jarun I'm not sure what you're referring to here… The album changes? The typo fix you've requested? Force-pushing all these changes in a single commit as you were asking in the dev discussion? …Either way, I'm suspecting you meant @rachmadaniHaryono as he's the author of this pull request 😅

(…If it's the typo, it appears to have been fixed in the latest commit.)

@LeXofLeviafan
Copy link
Collaborator

…I'm gonna leave this here to keep it visible, just in case.

I don't know the severity of this exploit

…Actually, now that I'm looking at the implementation, it doesn't even seem like the exploit is applicable in the first place. The browser never accesses those sites (nor even the internal favicon cache) when displaying these images; they're downloaded directly from a Google site (t2.gstatic.com), so the only one who's capable of doing "browser fingerprinting" on the user is Google itself (and let's be honest, they have much better tools for user tracking than mucking around with third-party images 😅). As for the exploit in question, all of these sources describe it as tracking actual site visitors; in our case, however, the sites won't even be aware that the user is trying to access their favicons – there's no reason for Google to explicitly support someone else's efforts at user tracking (which would take some actual effort and server traffic/CPU time for Google to make it work).

In short, the only place that receives these requests is a Google site, they're extremely unlikely to be tracked like this by that site, and pretty much guaranteed to not be tracked by the favicon owners. (And they're handled as regular images by the browser, so the favicon cache is never accessed either.)

@jarun
Copy link
Owner

jarun commented Jan 15, 2023

I am waiting for the following:

i will post picture here first and then upload it after agreed

@rachmadaniHaryono
Copy link
Collaborator Author

just add note from my program for developer to use buku, it is not final yet and could be formatted better

maybe follow how buku usage guide and crate python code equivalent

@jarun
Copy link
Owner

jarun commented Jan 23, 2023

Guys, please let me know when this is ready.

@LeXofLeviafan LeXofLeviafan mentioned this pull request Jan 24, 2023
71 tasks
@jarun
Copy link
Owner

jarun commented Feb 8, 2023

@LeXofLeviafan @rachmadaniHaryono can this be merged?

@LeXofLeviafan
Copy link
Collaborator

There's been no updates on the new gallery, screenshot margins, or the gallery/screenshot naming yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants