static-fe overhaul #236

Merged
floatingghost merged 24 commits from :pretty-staticfe into develop 2022-12-07 11:20:53 +00:00
First-time contributor

makes static-fe look more like pleroma-fe, with the stylesheets matching pleroma-dark and pleroma-light based on prefers-color-scheme.

  • navbar
  • about sidebar
  • background image
  • statuses
    • "reply to" or "edited" tags
  • accounts
    • show more / show less
    • posts / with replies / media / followers / following
      • followers/following would require user card snippets
    • admin/bot indicators
  • attachments
    • nsfw attachments
  • fontawesome icons
  • clean up and sort css
  • add pleroma-light
  • replace hardcoded strings

also i forgot

  • repeated headers

how it looks + sneak peek at statuses:

makes static-fe look more like pleroma-fe, with the stylesheets matching pleroma-dark and pleroma-light based on `prefers-color-scheme`. - [x] navbar - [x] about sidebar - [x] background image - [x] statuses - [x] "reply to" or "edited" tags - [x] accounts - [x] show more / show less - [x] posts / with replies / media / followers / following - [x] followers/following would require user card snippets - [x] admin/bot indicators - [x] attachments - [x] nsfw attachments - [x] fontawesome icons - [x] clean up and sort css - [x] add pleroma-light - [x] replace hardcoded strings also i forgot - [x] repeated headers how it looks + sneak peek at statuses: ![](https://akkoma.dev/attachments/c0d3a025-6987-4630-8eb9-5f4db6858359)
Ghost changed title from WIP: Add navbar, sidebar and background to static-fe to WIP: static-fe overhaul 2022-10-23 00:11:08 +00:00
Author
First-time contributor

profile view:

profile view:
Author
First-time contributor

"show newer" button seems to not work at all even in original static-fe.

"show newer" button seems to not work at all even in original static-fe.
Author
First-time contributor

nsfw attachments:

nsfw attachments: ![](https://akkoma.dev/attachments/cfbe752f-19a3-4ac5-89ba-49cee38c3083)
Author
First-time contributor

need help: separating the timelines into posts, with replies, media, and followers/following
i just don't know elixir or the codebase enough to know which functions to call

need help: separating the timelines into posts, with replies, media, and followers/following i just don't know elixir or the codebase enough to know which functions to call
Ghost force-pushed pretty-staticfe from abb6f00cdd to d5c0aac3b8 2022-11-18 12:20:38 +00:00 Compare
Author
First-time contributor

it'S DONE

it'S DONE
Ghost changed title from WIP: static-fe overhaul to static-fe overhaul 2022-11-18 15:01:23 +00:00
Ghost force-pushed pretty-staticfe from 091baf5d47 to e5ebb2ab20 2022-11-18 17:20:44 +00:00 Compare
Ghost force-pushed pretty-staticfe from e5ebb2ab20 to 4cc35924ea 2022-11-25 16:24:20 +00:00 Compare
Author
First-time contributor

i Think (tm) this works but my machine for some reason doesn't wanna do tests correctly 🥴

i Think (tm) this works but my machine for some reason doesn't wanna do tests correctly 🥴
Ghost changed title from static-fe overhaul to WIP: static-fe overhaul 2022-11-27 15:17:39 +00:00
Ghost force-pushed pretty-staticfe from 3e75178b61 to d494addf54 2022-11-27 16:05:23 +00:00 Compare
Ghost force-pushed pretty-staticfe from 011eb5bcaa to 2178c96ff6 2022-11-27 19:28:52 +00:00 Compare
Ghost changed title from WIP: static-fe overhaul to static-fe overhaul 2022-11-29 12:04:55 +00:00
Author
First-time contributor

ok Now it should be done but for real actually tm
working demo at snowdin.town

ok Now it should be done but for real actually tm working demo at snowdin.town
floatingghost reviewed 2022-11-29 19:02:53 +00:00
@ -26,0 +26,4 @@
defp no_static?(conn) do
conn.query_params
|> Map.has_key?("no_static")

we shouldn't allow people to change routing with query parameters

we shouldn't allow people to change routing with query parameters
Author
First-time contributor

okay so this was like a last minute update because people complained about how they can't move to the primary frontend from the backend, and pleroma-fe lies about your url, so there's no other way to figure out whether static-fe Should be loaded except for like. a cookie maybe?

i'm fine with removing this commit for the time being but it's a feature that should be added sometime

okay so this was like a last minute update because people complained about how they can't move to the primary frontend from the backend, and pleroma-fe lies about your url, so there's no other way to figure out whether static-fe Should be loaded except for like. a cookie maybe? i'm fine with removing this commit for the time being but it's a feature that should be added sometime
@ -726,1 +726,4 @@
scope "/", Pleroma.Web do
# Note: html format is supported only if static FE is enabled
pipe_through([:accepts_html_xml, :static_fe])

we're not posting, this scope doesn't need to take any Accept headers - you can remove the accepts

we're not posting, this scope doesn't need to take any `Accept` headers - you can remove the accepts
Ghost marked this conversation as resolved
@ -760,2 +769,3 @@
scope "/", Pleroma.Web.ActivityPub do
pipe_through([:activitypub_client])
# Note: html format is supported only if static FE is enabled
pipe_through([:activitypub_client, :accepts_html_xml_json, :static_fe])

why was this changed?

why was this changed?
Author
First-time contributor

fetching /users/:nickname/followers or following with Accept: application/activity+json should return the AP data, but with HTML it needs to return the static-fe list of follow(er)s

. i could take those two specifically though instead of the whole bunch

fetching `/users/:nickname/followers` or `following` with `Accept: application/activity+json` should return the AP data, but with HTML it needs to return the static-fe list of follow(er)s . i could take those two specifically though instead of the whole bunch
@ -56,10 +57,31 @@ defmodule Pleroma.Web.StaticFE.StaticFEController do
|> Map.take(@page_keys)
|> Map.new(fn {k, v} -> {String.to_existing_atom(k), v} end)
params =

you should probably have a hard limit on your params, else you can manually specify ?limit in your URL and make it apply

you should probably have a hard `limit` on your params, else you can manually specify `?limit` in your URL and make it apply
Ghost marked this conversation as resolved
floatingghost reviewed 2022-11-29 19:07:15 +00:00
@ -63,0 +77,4 @@
|> Enum.map(&represent/1)
"following" ->
User.get_friends(user)

you should honour user.hide_followers and user.hide_follows here

you should honour `user.hide_followers` and `user.hide_follows` here
Ghost marked this conversation as resolved
@ -153,0 +175,4 @@
reply_to_user =
if data["inReplyTo"] do
data["inReplyTo"]
|> Activity.get_create_by_object_ap_id()
this should never pull in data use https://akkoma.dev/AkkomaGang/akkoma/src/branch/develop/lib/pleroma/activity.ex#L291
Ghost marked this conversation as resolved
@ -153,0 +177,4 @@
data["inReplyTo"]
|> Activity.get_create_by_object_ap_id()
|> Map.get(:actor)
|> CommonAPI.get_user()

why are you using commonAPI here?

why are you using commonAPI here?
Ghost marked this conversation as resolved
@ -22,17 +21,44 @@ defmodule Pleroma.Web.StaticFE.StaticFEView do
Utils.fetch_media_type(@media_types, mediaType)
end
def time_ago(date) do

replace with https://hexdocs.pm/timex/Timex.html#from_now/1 instead of reinventing the wheel

replace with https://hexdocs.pm/timex/Timex.html#from_now/1 instead of reinventing the wheel
Ghost marked this conversation as resolved
floatingghost requested changes 2022-11-29 19:09:46 +00:00
floatingghost left a comment
Owner

it looks nice, but parts of this will be very difficult to maintain

mostly looking at SVG literals - these should absolutely be replaced

if you can manage that i'll take another look

it looks nice, but parts of this will be very difficult to maintain mostly looking at SVG literals - these should absolutely be replaced if you can manage that i'll take another look
@ -17,0 +14,4 @@
</a>
<div class="user-summary">
<div class="top-line">
<span class="username">

formatting is all over the place in this file

formatting is all over the place in this file
Ghost marked this conversation as resolved
Author
First-time contributor

yeah there's a lot of shit in here because i was doing this blind and mostly looking at similar things 🥴

yeah there's a lot of shit in here because i was doing this blind and mostly looking at similar things 🥴
Ghost force-pushed pretty-staticfe from 70fe8fb993 to ecfda57461 2022-12-01 15:48:47 +00:00 Compare
Ghost force-pushed pretty-staticfe from ecfda57461 to 12db8a40c5 2022-12-01 16:03:28 +00:00 Compare
Author
First-time contributor

two other things i have to add:

  • being on a remote user on pleroma-fe and reloading leads to a "user not found", need to add logic to extract the object and redirect there
  • jeder reminded me that there's no polls yet
  • 5bb95256ee fucked with img classes, emoji are now not discernable from other images, and they're incorrectly sized
two other things i have to add: - being on a remote user on pleroma-fe and reloading leads to a "user not found", need to add logic to extract the object and redirect there - jeder reminded me that there's no polls yet - 5bb95256ee2cd0609b6c821820f165a9e6b6c57f fucked with img classes, emoji are now not discernable from other images, and they're incorrectly sized
Ghost force-pushed pretty-staticfe from 12db8a40c5 to e4c9d57aa2 2022-12-02 11:48:14 +00:00 Compare
floatingghost reviewed 2022-12-02 12:16:25 +00:00
@ -102,2 +102,4 @@
"alt"
])
Meta.allow_tag_with_this_attribute_values(:img, "class", ["emoji"])

????????????????????????????????????????????????????????????????????????????????????????????????

????????????????????????????????????????????????????????????????????????????????????????????????
Author
First-time contributor

...yes?
there is literally nothing differentiating an emoji from an inline image except for the emoji tag here

https://akkoma.dev/AkkomaGang/akkoma/src/branch/develop/lib/pleroma/emoji/formatter.ex#L32 adds the emoji tag to emoji, but the changes in 5bb95256ee effectively reverses that, making it impossible to size emoji correctly without ruining all other images

...yes? there is literally nothing differentiating an emoji from an inline image except for the emoji tag here https://akkoma.dev/AkkomaGang/akkoma/src/branch/develop/lib/pleroma/emoji/formatter.ex#L32 adds the `emoji` tag to emoji, but the changes in https://akkoma.dev/AkkomaGang/akkoma/commit/5bb95256ee2cd0609b6c821820f165a9e6b6c57f effectively reverses that, making it impossible to size emoji correctly without ruining all other images

we should not be allowing literally any server on the network to pass classes through to the frontend

we should not be allowing literally any server on the network to pass classes through to the frontend
Author
First-time contributor

including our own? as far as i can tell emoji assignment is done on our side even for remote, could be wrong though

in any case how would you render any emoji without knowing what's an emoji

including our own? as far as i can tell emoji assignment is done on our side even for remote, could be wrong though in any case how would you render any emoji without knowing what's an emoji

"our own" is the whole reason that the leak happened in the first place

there is no distinction between "our" classes and what can be applied via image classes

"our own" is the whole reason that the leak happened in the first place there is _no distinction_ between "our" classes and what can be applied via image classes
Author
First-time contributor

no but what i mean is
maybe the classes could be scrubbed before our server adds emoji postprocessing
you can't inject classes in a :shortname:

no but what i mean is maybe the classes could be scrubbed *before* our server adds emoji postprocessing you can't inject classes in a `:shortname:`
Author
First-time contributor

again, unless remote servers send their emoji already processed and not in shortcode, in which case "aaa" and "fuck"

again, unless remote servers send their emoji already processed and not in shortcode, in which case "aaa" and "fuck"

remote servers are entirely allowed to send <img> tags

this change would allow those to pass classes through the scrubbe

remote servers are entirely allowed to send `<img>` tags this change would allow those to pass classes through the scrubbe
Author
First-time contributor

i guess a compromise would have to be.. non-emoji images given without width or height would by default be sized like emoji?

i guess a compromise would have to be.. non-emoji images given without width or height would by default be sized like emoji?
Author
First-time contributor

remote servers are entirely allowed to send <img> tags

this change would allow those to pass classes through the scrubbe

ok yes so
removing this commit and talking specifically about emoji

are they sent as <img> tags or as shortcodes? if the latter, you could first scrub the incoming post, then format it, thereby ending up with correctly classed emoji

if not then that's a different problem but i feel like there's potentially a really easy solution to this

> remote servers are entirely allowed to send `<img>` tags > > this change would allow those to pass classes through the scrubbe ok yes so removing this commit and talking specifically about emoji are they sent as `<img>` tags or as shortcodes? if the latter, you could first scrub the incoming post, *then* format it, thereby ending up with correctly classed emoji if not then that's a different problem but i feel like there's potentially a really easy solution to this
Author
First-time contributor

this is ugly and i don't like it but it works

img:not(.u-photo, .fa-icon) {
    width: 32px;
    height: 32px;
    padding: 0;
    vertical-align: middle;
}

.username img:not(.u-photo) {
    width: 16px;
    height: 16px;
}
this is ugly and i don't like it but it works ```css img:not(.u-photo, .fa-icon) { width: 32px; height: 32px; padding: 0; vertical-align: middle; } .username img:not(.u-photo) { width: 16px; height: 16px; } ```
Ghost marked this conversation as resolved
Ghost force-pushed pretty-staticfe from e4c9d57aa2 to ad76fcc673 2022-12-03 12:30:34 +00:00 Compare
Ghost force-pushed pretty-staticfe from 422b2a9a12 to efa3f1b71b 2022-12-03 13:32:23 +00:00 Compare
Ghost requested review from floatingghost 2022-12-03 14:41:29 +00:00
floatingghost reviewed 2022-12-03 23:01:04 +00:00
floatingghost left a comment
Owner

much more readable

a few cache things

you may also want to add some attribution for fontawesome (just a txt file in the static fe view will do) - they don't require it but they prefer it

much more readable a few cache things you may also want to add some attribution for fontawesome (just a txt file in the static fe view will do) - they don't require it but they prefer it
@ -724,6 +724,12 @@ defmodule Pleroma.Web.Router do
get("/users/:nickname/feed", Feed.UserController, :feed, as: :user_feed)
end
scope "/", Pleroma.Web do

this fragment can be refactored

scope "/", Pleroma.Web.StaticFE do ...

which allows you to remove the prefix in the routes

this fragment can be refactored ``` scope "/", Pleroma.Web.StaticFE do ... ``` which allows you to remove the prefix in the routes
Ghost marked this conversation as resolved
@ -153,0 +185,4 @@
activity
|> Activity.get_in_reply_to_activity()
|> Map.get(:actor)
|> User.get_by_ap_id()

use get_cached_by_ap_id

use `get_cached_by_ap_id`
Ghost marked this conversation as resolved
@ -24,1 +23,4 @@
def time_ago(date) do
{:ok, date, _} = DateTime.from_iso8601(date)
{:ok, now} = DateTime.now("Etc/UTC")

you can use DateTime.utc_now for this

you can use DateTime.utc_now for this
Ghost marked this conversation as resolved
Author
First-time contributor

i've also been trying to trim down the number of commits and make it more coherent what each commit does, but the css file doesn't wanna cooperate with all of the changes.

i've also been trying to trim down the number of commits and make it more coherent what each commit does, but the css file doesn't wanna cooperate with all of the changes.
Author
First-time contributor

you may also want to add some attribution for fontawesome (just a txt file in the static fe view will do) - they don't require it but they prefer it

they seem to actually not mind at all actually

Attribution is required by MIT, SIL OLF, and CC BY licenses. Downloaded Font Awesome Free files already contain embedded comments with sufficient attribution, so you shouldn't need to do anything additional when using these files normally.

> you may also want to add some attribution for fontawesome (just a txt file in the static fe view will do) - they don't require it but they prefer it they seem to actually not mind at all actually > Attribution is required by MIT, SIL OLF, and CC BY licenses. Downloaded Font Awesome Free files already contain embedded comments with sufficient attribution, so **you shouldn't need to do anything additional** when using these files normally.
Ghost force-pushed pretty-staticfe from daf5fc7e78 to 9ed1398651 2022-12-03 23:53:06 +00:00 Compare
Ghost force-pushed pretty-staticfe from 9ed1398651 to 7063631787 2022-12-03 23:57:54 +00:00 Compare
Author
First-time contributor

@floatingghost (this is ready to review i just cant seem to ask for review again)

@floatingghost (this is ready to review i just cant seem to ask for review again)

this seems fine now!

thank you very much, this is... massive - finally a functional static-fe

this seems fine now! thank you very much, this is... massive - finally a functional static-fe
floatingghost merged commit 7c4b415929 into develop 2022-12-07 11:20:53 +00:00
floatingghost deleted branch pretty-staticfe 2022-12-07 11:20:53 +00:00
floatingghost referenced this pull request from a commit 2022-12-07 11:20:54 +00:00
rat referenced this pull request from a commit 2024-02-25 20:33:30 +00:00
Sign in to join this conversation.
No description provided.