[WIP] #8374: Limit post content by rendered height instead of number of characters #968

Merged

wiwie merged 19 commits from develop into develop

2020-03-18 01:48:19 +01:00

wiwie commented

2020-03-12 23:26:59 +01:00

(Migrated from github.com)

Initial implementation as addon.

👍 1 🎉 1 🚀 1

MrPetovan (Migrated from github.com) requested changes 2020-03-13 03:43:09 +01:00

MrPetovan (Migrated from github.com) left a comment

Thank you for your work, here's my feedback.

wiwie (Migrated from github.com) reviewed 2020-03-13 12:09:57 +01:00

wiwie (Migrated from github.com) reviewed 2020-03-13 12:11:09 +01:00

wiwie (Migrated from github.com) reviewed 2020-03-13 12:16:28 +01:00

wiwie commented

2020-03-13 23:17:02 +01:00

(Migrated from github.com)

If I wanted to make the limit-height a addon setting; how would I go about that? I can't really figure out how I could inject the setting value into the JS and CSS files.

MrPetovan commented

2020-03-14 03:36:43 +01:00

(Migrated from github.com)

I can see two ways:

Make the JS file a dynamic templates and serving it from the addon as a module.
Injecting a small declarative piece of Javascript in the page head on top of the registerFooterScript that will reference the variable you will have declared.

I can see two ways: - Make the JS file a dynamic templates and serving it from the addon as a module. - Injecting a small declarative piece of Javascript in the page head on top of the registerFooterScript that will reference the variable you will have declared.

wiwie commented

2020-03-14 21:38:18 +01:00

(Migrated from github.com)

Ok, I can't figure out either of them; are there any examples for either approach?
(1) Does this mean to implement showmore_dyn_module()/showmore_dyn_content() and then somehow adding /showmore_dyn/X to the page header/footer?
(2) The registerFooterScript only takes a path to a JS file; if I'm getting you right, you mean I'd write the JS to a string and somehow add it to the dynamically in the code?

Ok, I can't figure out either of them; are there any examples for either approach? (1) Does this mean to implement showmore_dyn_module()/showmore_dyn_content() and then somehow adding /showmore_dyn/X to the page header/footer? (2) The registerFooterScript only takes a path to a JS file; if I'm getting you right, you mean I'd write the JS to a string and somehow add it to the dynamically in the code?

MrPetovan commented

2020-03-14 21:44:56 +01:00

(Migrated from github.com)

You got it. Although I don't recommend it for performance purposes.
It's as simple as DI::page()['htmlhead'] .= '<script>var postHeightLimit = ' . intval($configValue) . ';</script>';

1. You got it. Although I don't recommend it for performance purposes. 2. It's as simple as `DI::page()['htmlhead'] .= '<script>var postHeightLimit = ' . intval($configValue) . ';</script>';`

wiwie commented

2020-03-14 21:49:24 +01:00

(Migrated from github.com)

great, got it - thanks!

👍 1

wiwie commented

2020-03-14 23:38:19 +01:00

(Migrated from github.com)

Addressed all your comments; except your question regarding "is wrapper needed". Are you ok with leaving it as is?

MrPetovan (Migrated from github.com) requested changes 2020-03-15 05:10:57 +01:00

MrPetovan (Migrated from github.com) left a comment

More feedback, for each code standards requests please check the whole file as I didn't repeat them for multiple instances.

MrPetovan (Migrated from github.com) reviewed 2020-03-15 05:12:03 +01:00

wiwie commented

2020-03-15 10:35:44 +01:00

(Migrated from github.com)

Is there a good IDE I can use that does the source formatting automatically? I've been using vim so far but it's a pain to do this manually...

I'm used to eclipse for other programming languages, so that would be the easiest if I could just somehow import friendica as a new project.

Is there a good IDE I can use that does the source formatting automatically? I've been using vim so far but it's a pain to do this manually... I'm used to eclipse for other programming languages, so that would be the easiest if I could just somehow import friendica as a new project.

tobiasd commented

2020-03-15 12:11:18 +01:00

(Migrated from github.com)

If you install the editorconfig and the vim addon, then the basic stuff of formatting will be done automatically. But the enclosure of oberators with spaces e.g. not. There are linter addons for vim as well, which wont do it automatically, but tell you about code problems while in visual mode.

👍 1

wiwie (Migrated from github.com) reviewed 2020-03-15 12:24:15 +01:00

wiwie commented

2020-03-15 13:44:08 +01:00

(Migrated from github.com)

Addressed all comments.

I used the vim Autoformat plugin and double checked all formatting issues. I hope I didn't miss any...

Addressed all comments. I used the vim Autoformat plugin and double checked all formatting issues. I hope I didn't miss any...

MrPetovan commented

2020-03-15 15:36:26 +01:00

(Migrated from github.com)

If you did I'll be sure to mention them!

🎉 1 😆 1

MrPetovan (Migrated from github.com) requested changes 2020-03-15 15:45:43 +01:00

MrPetovan (Migrated from github.com) reviewed 2020-03-16 15:26:50 +01:00

MrPetovan (Migrated from github.com) reviewed 2020-03-17 07:14:02 +01:00

MrPetovan (Migrated from github.com) requested changes 2020-03-17 07:17:34 +01:00

MrPetovan (Migrated from github.com) left a comment

Please remove the wrapper.

Additionally, the height limit shouldn't trigger when viewing a single thread(/display/...). I suggest you check for the URL pattern since the HTML will vary greatly between themes.

Please remove the wrapper. Additionally, the height limit shouldn't trigger when viewing a single thread(`/display/...`). I suggest you check for the URL pattern since the HTML will vary greatly between themes.

wiwie commented

2020-03-17 19:57:26 +01:00

(Migrated from github.com)

Edit: Okay, I think I misunderstood you. You suggested to add the toggle div directly to the body div, right? This is what I now tried, and - to my surprise - it works flawlessly. CSS is and will always be a great mystery to me ;-) Plus, I could throw out about half of the CSS statements and they didn't change a thing.

Okay. Are you confident that your approach works for all themes and all cases? So is it always like that that there is nothing else added to the content div after the body div? Because otherwise the show more would be placed too low and not at the bottom of the content body. Edit: Okay, I think I misunderstood you. You suggested to add the toggle div directly to the body div, right? This is what I now tried, and - to my surprise - it works flawlessly. CSS is and will always be a great mystery to me ;-) Plus, I could throw out about half of the CSS statements and they didn't change a thing.

MrPetovan (Migrated from github.com) approved these changes 2020-03-18 01:45:50 +01:00

MrPetovan commented

2020-03-18 01:48:10 +01:00

(Migrated from github.com)

Thank you for your work! Since we are in Release Candidate period, this addon will be officially released in June with the Friendica 2020.06 version. Admins running the develop branch will be able to enable the addon sooner.

Thank you for your work! Since we are in Release Candidate period, this addon will be officially released in June with the Friendica 2020.06 version. Admins running the `develop` branch will be able to enable the addon sooner.

👍 1

wiwie commented

2020-03-18 09:31:23 +01:00

(Migrated from github.com)

Thanks for your help 😊

Thanks for your help :blush:

❤️ 1