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

Image refactor refactor #541

Merged
merged 8 commits into from
Oct 3, 2023
Merged

Conversation

ekzyis
Copy link
Member

@ekzyis ekzyis commented Oct 3, 2023

  • Removed comments about srcSet which no longer made sense
  • Update comments in <ImageOriginal>
  • Simplify condition when image is loaded in <ImageOriginal>
  • Rename column from clickToLoadImg to imgproxyOnly since we no longer use clicking to bypass setting. We simply show the link for the user to click. (I also never liked the name of the setting)
  • Add info about setting in /settings
  • Fix misleading URL on imgproxy errors: I read your comment here and will make sure it supports markdown links, too

@huumn
Copy link
Member

huumn commented Oct 3, 2023

😂 the title

@ekzyis ekzyis marked this pull request as draft October 3, 2023 14:15
@ekzyis ekzyis marked this pull request as ready for review October 3, 2023 14:58
@ekzyis ekzyis marked this pull request as draft October 3, 2023 15:07
@ekzyis
Copy link
Member Author

ekzyis commented Oct 3, 2023

Mhh, or wait. I have a better solution:

If [text](url) is used and text is not empty, I think we shouldn't render this as an image at all.

This would also fix the markdown in my bio. I use [click here](https://ekzyis.com/simplex.jpeg) there but it displays the image which was precisely what I tried to avoid.

edit: Also 19d2f5f throws an errors on [![2023-10-03-150917-1920x1080-scrot.png](https://i.postimg.cc/d31HNctn/2023-10-03-150917-1920x1080-scrot.png)](https://postimg.cc/34sFW6GD) as the input (but apparently not consistently 🤔)

@ekzyis ekzyis force-pushed the image-refactor-refactor branch 2 times, most recently from 9cdbc24 to 78a6167 Compare October 3, 2023 15:53
ekzyis added 7 commits October 3, 2023 17:55
We no longer distinguish between `undefined` and `null` for `srcSet`.
I think this is not required since `showImage` will always stay false if tab !== 'preview' or me?.clickToLoadImg are true.
@ekzyis ekzyis force-pushed the image-refactor-refactor branch from 78a6167 to 2097399 Compare October 3, 2023 15:55
@ekzyis ekzyis marked this pull request as ready for review October 3, 2023 15:55
@ekzyis ekzyis force-pushed the image-refactor-refactor branch from 2097399 to 967651b Compare October 3, 2023 15:57
@huumn
Copy link
Member

huumn commented Oct 3, 2023

Also 19d2f5f throws an errors on 2023-10-03-150917-1920x1080-scrot.png as the input (but apparently not consistently 🤔

Is this good to merge or does this need to be fixed?

@ekzyis
Copy link
Member Author

ekzyis commented Oct 3, 2023

Fixed in 9b204ae using optional chaining: children?.[0]

So this is good to merge now

@huumn huumn merged commit 1e417ba into stackernews:master Oct 3, 2023
1 check passed
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.

2 participants