FreeTube icon indicating copy to clipboard operation
FreeTube copied to clipboard

Fix/improve community post list styling

Open kommunarr opened this issue 2 years ago • 14 comments

Improve community post styling & fix list styling

Pull Request Type

  • [x] Bugfix

Related issue

closes #3984

Description

Fixes community post styling on the list view (as in, is legible and actually pretty). Wraps each community post in its own ft-card for list and grid views, and equalizes the list and grid views (given that there does not seem to be a good way to actually display community posts in a grid).

Screenshots

Screenshot_20230902_021401 localhost_9080_(iPad Air) localhost_9080_(iPhone 12 Pro)

Testing

1, Under General Settings, set Video View Type to "List" 2. Navigate to this channel 3. Navigate to the Community tab 4. Ensure that view appears correctly 5. Check with smaller device size (Chrome devtool) 6. Repeat for Grid view 7. Check with smaller device size (Chrome devtool)

Desktop

  • OS: OpenSUSE
  • OS Version: Tumbleweed
  • FreeTube version: 0.19.0

kommunarr avatar Sep 02 '23 07:09 kommunarr

Community posts were only ever meant to be displayed in a list view, but I'm guessing it was only ever tested with global grid, forced listed, not global list + forced list.

That's why he have the display list option.

absidue avatar Sep 02 '23 09:09 absidue

I guess the question is, instead of letting the the ft-community-post component even accept a grid and list value, maybe just doing it's own thing always in list mode would require less workarounds? (can this be fixed in the ft-community-post component without requiring "global" overrides?)

Especially requiring the community-posts list on the channel page to be almost at the root, is going to make it impossible to create a shared tab component, because it can't be in a normal nested layout anymore.

absidue avatar Sep 02 '23 09:09 absidue

I guess the question is, instead of letting the the ft-community-post component even accept a grid and list value, maybe just doing it's own thing always in list mode would require less workarounds?

Yeah I was of two minds about this - it technically isn't doing any good right now, but I thought I'd keep it this way in case someone does have a good way of differentiating between the two styles (+ didn't know if overriding the user's list/grid preference would be a bad idea to start doing). The only difference is that a handful of CSS properties have been added to the grid view version to equalize it. But I don't really care much either way, so I can remove that if we have any reason to at all.

(can this be fixed in the ft-community-post component without requiring "global" overrides?) Especially requiring the community-posts list on the channel page to be almost at the root, is going to make it impossible to create a shared tab component, because it can't be in a normal nested layout anymore.

Is this in reference to the putting it outside of the ft-card object covering a lot of the tabs in Channel.vue? Yeah, this one was giving me a lot of decision grief - I think it's nice to have a way to allow these ft-element-lists to be wrapped in ft-cards, but taking it out of there is messier for sure. How about this: I put it back in there, and add styling to ft-card.css that overrides the parent styling if it has any ft-card children (with a certain class) (not sure if :has has been implemented yet at our Electron version, but I'll check)?

kommunarr avatar Sep 02 '23 14:09 kommunarr

We already override the preference in a few places, e.g. the playlists page. The community posts were always meant to override the users global choice (that's why the display="list" prop exists), it was just only made for when the global choice is grid, which is why it worked fine for that but not list.

I partially fixed it a while back, by making the community posts think that the global choice was always grid, but never got round to fully fixing it.

absidue avatar Sep 02 '23 14:09 absidue

We already override the preference in a few places, e.g. the playlists page. The community posts were always meant to override the users global choice (that's why the display="list" prop exists), it was just only made for when the global choice is grid, which is why it worked fine for that but not list.

I partially fixed it a while back, by making the community posts think that the global choice was always grid, but never got round to fully fixing it.

Ah okay, I was thinking that to some extent when I saw it at the top level, but then I saw we were still grabbing their preference on the ft-community-post page and was confused. I can just remove it from the community posts directly then as you've said. Thanks!

kommunarr avatar Sep 02 '23 14:09 kommunarr

Yeah it's a bit weird, the list has to be in list layout but the post itself, has to be in grid layout for it to work well.

absidue avatar Sep 02 '23 14:09 absidue

I was able to get the community posts to look identical in both the list and grid global layout, with these simple changes. ~~Maybe I'm misunderstanding what this pull request is supposed to achieve, but it seems like it might be overcomplicating this rather simple change (modifying the way ft-card works and using component is, seems like the wrong approach to me).~~

Edit: Discussed on Matrix, the original comment made incorrect assumptions, there is more to the pull request than just this, but we managed to discuss a potential way of avoiding so many changes

diff --git a/src/renderer/components/ft-community-post/ft-community-post.js b/src/renderer/components/ft-community-post/ft-community-post.js
index a7086c29..c7d1c7f7 100644
--- a/src/renderer/components/ft-community-post/ft-community-post.js
+++ b/src/renderer/components/ft-community-post/ft-community-post.js
@@ -52,10 +52,6 @@ export default defineComponent({
         slideBy: 'page',
         navPosition: 'bottom'
       }
-    },
-
-    listType: function () {
-      return this.$store.getters.getListType
     }
   },
   created: function () {
diff --git a/src/renderer/components/ft-community-post/ft-community-post.vue b/src/renderer/components/ft-community-post/ft-community-post.vue
index 870a6680..d528af50 100644
--- a/src/renderer/components/ft-community-post/ft-community-post.vue
+++ b/src/renderer/components/ft-community-post/ft-community-post.vue
@@ -1,9 +1,8 @@
 <template>
   <div
     v-if="!isLoading"
-    class="ft-list-post ft-list-item outside"
+    class="ft-list-post ft-list-item outside grid"
     :appearance="appearance"
-    :class="{ list: listType === 'list', grid: listType === 'grid' }"
   >
     <div
       class="author-div"

absidue avatar Sep 03 '23 22:09 absidue

So it's doing a bit more than that if you see the picture and description; but I will remove the complicating stuff.

kommunarr avatar Sep 04 '23 13:09 kommunarr

This pull request has conflicts, please resolve those before we can evaluate the pull request.

github-actions[bot] avatar Sep 09 '23 16:09 github-actions[bot]

Conflicts have been resolved. A maintainer will review the pull request shortly.

github-actions[bot] avatar Oct 08 '23 02:10 github-actions[bot]

This pull request has conflicts, please resolve those before we can evaluate the pull request.

github-actions[bot] avatar Dec 08 '23 19:12 github-actions[bot]

Is there anything blocking this except for the conflicts?

Conflicts have been resolved. A maintainer will review the pull request shortly.

github-actions[bot] avatar Jan 05 '24 03:01 github-actions[bot]

Community posts are still broken, I think, so that needs to be handled

kommunarr avatar Jan 05 '24 03:01 kommunarr

This pull request has conflicts, please resolve those before we can evaluate the pull request.

github-actions[bot] avatar Jan 22 '24 23:01 github-actions[bot]

This one is a nice lil visual improvement for all users, and it does fix Community Post styling for people who use List view, but honestly, I haven't heard a single soul complain about this, meaning very few people probably actually use List view (and/or read community posts on FreeTube). Too low priority for me to care about fixing compared to other items on my list.

kommunarr avatar Apr 20 '24 14:04 kommunarr