terrastories icon indicating copy to clipboard operation
terrastories copied to clipboard

Add story pinned option

Open griseduardo opened this issue 1 year ago • 5 comments

Motivation

Allow pin story and show first

Changes

  • Add story_pinned column in stories table
  • Allow editor and admin user to pin or unpin stories in stories dashboard and edit story dashboard
  • When a story is pinned, any other previous pinned is unpinned
  • Member user can only see the pin in pinned story but not update it
  • In home and dashboard show the pinned story first (if filters satisfy the condition to this story appear)

Screenshots

Stories dashboard (admin/editor): admin-editor-stories

Stories dashboard (member): user-stories

Story dashboard (admin/editor): edit-story

Home page: home-page

griseduardo avatar Oct 12 '23 19:10 griseduardo

This is great to see! Thanks so much.

A couple of things:

  1. The addition of these css assets are introducing a lot of unwanted styling artifacts on both the Rails dashboard and React sides.
    <link href="//netdna.bootstrapcdn.com/twitter-bootstrap/2.3.2/css/bootstrap-combined.no-icons.min.css" rel="stylesheet">
    <link href="//netdna.bootstrapcdn.com/font-awesome/3.2.1/css/font-awesome.css" rel="stylesheet">

You can see this in the blue underlines in your GIFs above, as well as the the New button at the top right which is supposed to be orange background, and a bunch of other little things. And on React, the light blue link color at the bottom.

Can we find a better way to access the icons without importing a bunch of CSS we either don't need or is impacting the app style?

  1. For a story that's been "Pinned" can we add a permanently visible pin icon at the top right?

rudokemper avatar Oct 13 '23 15:10 rudokemper

This is great to see! Thanks so much.

A couple of things:

1. The addition of these css assets are introducing a lot of unwanted styling artifacts on both the Rails dashboard and React sides.
    <link href="//netdna.bootstrapcdn.com/twitter-bootstrap/2.3.2/css/bootstrap-combined.no-icons.min.css" rel="stylesheet">
    <link href="//netdna.bootstrapcdn.com/font-awesome/3.2.1/css/font-awesome.css" rel="stylesheet">

You can see this in the blue underlines in your GIFs above, as well as the the New button at the top right which is supposed to be orange background, and a bunch of other little things. And on React, the light blue link color at the bottom.

Can we find a better way to access the icons without importing a bunch of CSS we either don't need or is impacting the app style?

2. For a story that's been "Pinned" can we add a permanently visible pin icon at the top right?

Hello, the problem is that the exact "icon-pushpin" is only presented in version 3 of font-awesome: https://fontawesome.com/v3/icons/ It is a version that is no longer supported and it is only possible to use with bootstrap (like I tried before) or downloading a folder with font-awesome configs, that brings all icons from there (not separated by icon). The icons presented in version 5/6 of font-awesome, they permit download the svg of free ones and they give support for them. I removed the bootstrap inclusion on dashboard and react head, downloaded the similar icon of pushpin on version 6 and used it. I updated the gifs in description to show how the icon appears. What do you think about use this svg? I updated to always show pin icon when the story is pinned

griseduardo avatar Oct 13 '23 20:10 griseduardo

@griseduardo You think you can rebase your branch so we dont have the conflicts?

julinvictus avatar Nov 02 '23 02:11 julinvictus

Thanks for working through all of the feedback! This is looking great. A couple of additional comments in line.

I would also love to see some specs around the pinning and unpinning actions — both the individual pin/unpin endpoints as well as in the create and edit form. Are you able to add those for us?

I will add the tests :)

griseduardo avatar Dec 12 '23 16:12 griseduardo

Hello @rudokemper and @lauramosher I believe that now is ready to re-review, I worked in all suggestions. The resolved ones I resolve here in pr, some I commented the implementation I followed and the reason :)

griseduardo avatar Dec 16 '23 17:12 griseduardo