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.

Sign up for GitHub

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

Add story pinned option #975

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open

Add story pinned option #975

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
2db93f2
Add story pinned
griseduardo Oct 12, 2023
b05935b
Refactor story pinned related files
griseduardo Oct 12, 2023
b41f09f
Adjust story pinned update in stories controller
griseduardo Oct 12, 2023
b377d61
Remove bootstrap inclusion in React and dashboard head, update to use…
griseduardo Oct 13, 2023
e5b3fff
Merge branch master into ge-add-story-pinned
griseduardo Nov 2, 2023
f4eb8fe
Refactor stories page order rules
griseduardo Nov 8, 2023
60ba313
Remove old stories page order rule
griseduardo Nov 8, 2023
3d2330b
Add pin and unpin actions for update stories
griseduardo Nov 12, 2023
26f0346
Add thumbstack to shared icons, update fill color behavior on icon pi…
griseduardo Nov 15, 2023
9b02f27
Refactor icon unpinned and icon pinned css
griseduardo Nov 16, 2023
e5220c7
Put edit story icon pinned inside form
griseduardo Nov 17, 2023
dda31ff
Update icon pinned svg call in React
griseduardo Nov 17, 2023
14f63b0
Update icon pin from font-awesome to google svg
griseduardo Nov 22, 2023
67b5eb3
Add pin and unpin stories tests for all actions related
griseduardo Dec 12, 2023
83b61c8
Move icons css to icons scss file
griseduardo Dec 12, 2023
86054dd
Remove not used card css definition
griseduardo Dec 12, 2023
d4d7432
Update stories pin and unpin form and button
griseduardo Dec 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions rails/app/assets/images/thumbtack-not-pinned.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions rails/app/assets/images/thumbtack-pinned.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
26 changes: 26 additions & 0 deletions rails/app/assets/stylesheets/cms/_cards.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -117,3 +117,29 @@
word-wrap: break-word;
}
}

.edit-icon-pinned, .edit-icon-not-pinned, .view-icon-pinned {
float: right;
height: fit-content;
margin-inline-start: auto;
}
griseduardo marked this conversation as resolved.
Show resolved Hide resolved

.edit-icon-pinned, .view-icon-pinned {
color: $black;
griseduardo marked this conversation as resolved.
Show resolved Hide resolved
display: flex;
}

.edit-icon-not-pinned {
color: $light-gray;
display: none;
}

.edit-icon-pinned, .edit-icon-not-pinned {
&:hover {
box-shadow: 0 0 10px $lighter-gray;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would love to see these use the SVG coloring system and moved into the icons style sheet and shared. Something like:

.icon-button {
  max-width: 25px;
  max-height: 25px;
  border: none;
  background-color: inherit;
}

.icon-unpinned {
  visibility: hidden;
  max-height: 18px;
  fill: $light-gray;

  &:hover {
    fill: $dark-gray;
    cursor: pointer;
  }
}

.icon-pinned {
  max-height: 18px;
  fill: $black;

  &:hover {
    fill: $red; // shows that clicking this would be a destructive action
    cursor: pointer;
  }
}

and then in the views, they can be referenced like so:

<%= f.button nil, class: "icon-button" do %>
  <svg class="icon-pinned"><svg content></svg>
<% end %>

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome. I just updated a bit because this way the color only changed when the mouse is on icon, but on button doesn't change the color of icon.
I updated to change the color of icon when the mouse is on the button


.card:hover .edit-icon-not-pinned {
display: flex;
}
griseduardo marked this conversation as resolved.
Show resolved Hide resolved
13 changes: 12 additions & 1 deletion rails/app/assets/stylesheets/cms/_icons.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,15 @@
&::before {
content: "\1F4CD";
}
}
}

.icon-pinned, .icon-not-pinned {
background-color: inherit;
border: none;
cursor: pointer;
height: fit-content;
}

.icon-not-pinned {
color: $light-gray;
}
griseduardo marked this conversation as resolved.
Show resolved Hide resolved
21 changes: 21 additions & 0 deletions rails/app/assets/stylesheets/cms/layout.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,3 +172,24 @@ svg[hidden] {
svg:not(:root) {
overflow: hidden;
}

.story-main-heading {
width: max-content;
}

.story-title {
display: flex;
}

.story-pinned {
align-items: center;
display: flex;
height: 100%;
float: right;
margin-left: 15px;
}

.image-pinned {
height: 15px;
width: 9px;
}
griseduardo marked this conversation as resolved.
Show resolved Hide resolved
8 changes: 8 additions & 0 deletions rails/app/assets/stylesheets/components/card.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,14 @@ $el-shadow: 0 1px 4px $lighter-gray;
.stories {
overflow-x: hidden;
flex: 5 0 0;
.pinned {
float: right;

img {
height: 30px;
width: 10px;
}
};
.container {
padding-bottom: 3rem;

Expand Down
24 changes: 23 additions & 1 deletion rails/app/controllers/dashboard/stories_controller.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,8 +43,29 @@ def edit

def update
@story = authorize community_stories.find(params[:id])
@stories = community_stories.all

if @story.update(story_params.except(:media))
if params[:story_pinned].present?
if params[:story_pinned]
@pinned_story = @stories.find_by(story_pinned: true)
@pinned_story&.update(story_pinned: !params[:story_pinned])
end

@story.update(story_pinned: params[:story_pinned])

return redirect_to action: "index"
end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For ease of maintaining and reducing complexity of this, I think we should split out this block to allow pinning/unpinning from non-form views as its own custom rails action. I might also suggest we create two, one for pinning and one for unpinning and trigger them using remote: true calls so we don't refresh the page on them. Another benefit of this will ensure the logic stays relatively simple:

def pin
  story = authorize community_stories.find(params[:id])
  community_stories.update_all(story_pinned: false)
  story.update(story_pinned: true)
end

def unpin
  story = authorize community_stories.find(params[:id])
  story.update(story_pinned: false)
end

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea separate in different actions.
Only one problem, related to use remote true, is that it doesn't change the icon color and reordenize the stories, that can pass the wrong idea that the pin or unpin action didn't worked.
I maintained the render behavior. What do you think about it?


if story_params[:story_pinned].present?
if story_params[:story_pinned]
@pinned_story = @stories.find_by(story_pinned: true)
@pinned_story&.update(story_pinned: !story_params[:story_pinned])
end

@story.update(story_pinned: story_params[:story_pinned])

render :edit
griseduardo marked this conversation as resolved.
Show resolved Hide resolved
elsif @story.update(story_params.except(:media))
story_params[:media]&.each do |media|
m = @story.media.create(media: media)
end
Expand Down Expand Up @@ -88,6 +109,7 @@ def story_params
:topic,
:interview_location_id,
:interviewer_id,
:story_pinned,
media: [],
speaker_ids: [],
place_ids: []
Expand Down
4 changes: 3 additions & 1 deletion rails/app/javascript/components/App.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,8 @@ class App extends Component {
ne_boundary_lat: PropTypes.string,
ne_boundary_long: PropTypes.string,
pitch: PropTypes.number,
bearing: PropTypes.string
bearing: PropTypes.string,
thumbtack_path: PropTypes.string
};


Expand Down Expand Up @@ -382,6 +383,7 @@ class App extends Component {
handleFilterCategoryChange={this.handleFilterCategoryChange}
handleFilterItemChange={this.handleFilterItemChange}
itemOptions={this.state.itemOptions}
thumbtack_path={this.props.thumbtack_path}
/>
<IntroPopup />
</React.Fragment>
Expand Down
4 changes: 3 additions & 1 deletion rails/app/javascript/components/Card.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,8 @@ class Card extends Component {
filterItem: PropTypes.string,
handleFilterCategoryChange: PropTypes.func,
handleFilterItemChange: PropTypes.func,
itemOptions: PropTypes.array
itemOptions: PropTypes.array,
thumbtack_path: PropTypes.string
};

static defaultProps = {
Expand Down Expand Up @@ -93,6 +94,7 @@ class Card extends Component {
handleFilterCategoryChange={this.props.handleFilterCategoryChange}
handleFilterItemChange={this.props.handleFilterItemChange}
itemOptions={this.props.itemOptions}
thumbtack_path={this.props.thumbtack_path}
/>

<div className="card--tasks">
Expand Down
6 changes: 6 additions & 0 deletions rails/app/javascript/components/Sort.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ class Sort extends Component {
return {value: value, label: this.props.t(value)};
}

storyPinnedSort = (sortedStories) => (
sortedStories.sort(story => story.story_pinned ? -1 : 1)
)

handleSort = (option) => {
this.setState({
sortSelectValue: option.value
Expand Down Expand Up @@ -93,6 +97,8 @@ class Sort extends Component {
}
}

sortedStories = this.storyPinnedSort(sortedStories)

this.props.handleStoriesChanged(sortedStories);
}

Expand Down
33 changes: 23 additions & 10 deletions rails/app/javascript/components/Story.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import React from "react";
import PropTypes from "prop-types";
import StoryMedia from "./StoryMedia";
import { useTranslation } from 'react-i18next';
import StoryMedia from "./StoryMedia";

const Story = props => {
const { t } = useTranslation();
const { story, storyClass } = props;
const { onStoryClick, story, storyClass, thumbtack_path } = props;

const renderSpeakers = speakers => {
return (
Expand Down Expand Up @@ -34,14 +34,21 @@ const Story = props => {
}

return (
<React.Fragment>
<>
<li
className={storyClass}
onClick={() => props.onStoryClick(story)}
onKeyDown={() => props.onStoryClick(story)}
onClick={() => onStoryClick(story)}
onKeyDown={() => onStoryClick(story)}
key={story.id}
role="presentation"
>
{
story.story_pinned && (
<div className="pinned">
<img src={thumbtack_path} alt="pinned-story" />
</div>
)
}
<div className="speakers">
{renderSpeakers(story.speakers)}
</div>
Expand All @@ -61,14 +68,20 @@ const Story = props => {
))
}
{
story.language &&
<p>
<b>{t("language")}:</b> {story.language}
</p>
story.language && (
<p>
<b>
{t("language")}
:
</b>
{' '}
{story.language}
</p>
)
}
</div>
</li>
</React.Fragment>
</>
);
}

Expand Down
4 changes: 3 additions & 1 deletion rails/app/javascript/components/StoryList.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ class StoryList extends Component {
filterItem: PropTypes.string,
handleFilterCategoryChange: PropTypes.func,
handleFilterItemChange: PropTypes.func,
itemOptions: PropTypes.array
itemOptions: PropTypes.array,
thumbtack_path: PropTypes.string
};

handleClickStory = (story, index) => {
Expand Down Expand Up @@ -55,6 +56,7 @@ class StoryList extends Component {
onStoryClick={this.props.onStoryClick}
storyClass={storyClass}
key={key}
thumbtack_path={this.props.thumbtack_path}
/>
);
};
Expand Down
1 change: 1 addition & 0 deletions rails/app/models/story.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ def public_points
# desc :text
# language :string
# permission_level :integer
# story_pinned :boolean default(FALSE)
# title :string
# topic :string
# created_at :datetime not null
Expand Down
8 changes: 5 additions & 3 deletions rails/app/pages/stories_page.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ def initialize(scoped_stories, meta = {})

@meta[:limit] ||= 10
@meta[:offset] ||= 0
@meta[:sort_by] ||= "created_at"
@meta[:sort_by_pinned] ||= "story_pinned"
@meta[:sort_by_created] ||= "created_at"
griseduardo marked this conversation as resolved.
Show resolved Hide resolved
@meta[:sort_dir] ||= "desc"
end

Expand All @@ -16,6 +17,7 @@ def relation
stories = stories.joins(:speaker_stories).where(speaker_stories: {speaker_id: @meta[:speaker]}) if @meta[:speaker].present?
stories = stories.where(permission_level: @meta[:visibility]) if @meta[:visibility].present?

stories.order(@meta[:sort_by] => @meta[:sort_dir])
stories = stories.order(@meta[:sort_by_pinned] => @meta[:sort_dir])
griseduardo marked this conversation as resolved.
Show resolved Hide resolved
stories.order(@meta[:sort_by_created] => @meta[:sort_dir])
end
end
end
38 changes: 34 additions & 4 deletions rails/app/views/dashboard/stories/_stories.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,40 @@
<% @stories.each do |story| %>
<%= link_to story, class: "card" do %>
<div>
<h3>
<span class="card__heading--small"><%= story.topic %></span>
<%= story.title %>
</h3>
<div style="display:flex">
<div>
<h3 style="width:max-content">
<span class="card__heading--small"><%= story.topic %></span>
<%= story.title %>
</h3>
</div>
<% if current_user.member? && story.story_pinned %>
<div class="view-icon-pinned">
<%= image_tag 'thumbtack-pinned.svg', alt: 'pinned-story', class: "image-pinned" %>
</div>
<% end %>
<% if current_user.admin? || current_user.editor? %>
<% if story.story_pinned %>
<div class="edit-icon-pinned">
<%= form_with model: @story, url: story_path(story.id), method: "patch", multipart: true, class: "form", data: {remote: false}, locale: true do |f| %>
<%= f.hidden_field :story_pinned, value: false %>
<%= f.button nil, class: "icon-pinned" do %>
<%= image_tag 'thumbtack-pinned.svg', alt: 'pinned-story', class: "image-pinned" %>
<% end %>
<% end %>
</div>
<% else %>
<div class="edit-icon-not-pinned">
<%= form_with model: @story, url: story_path(story.id), method: "patch", multipart: true, class: "form", data: {remote: false}, locale: true do |f| %>
<%= f.hidden_field :story_pinned, value: true %>
<%= f.button nil, class: "icon-not-pinned" do %>
<%= image_tag 'thumbtack-not-pinned.svg', alt: 'pinned-story', class: "image-pinned" %>
<% end %>
<% end %>
</div>
<% end %>
<% end %>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One of the benefits of using an SVG over a standard image is the ability to store them as code rather than image assets. In our views, we have a shared/_icons.html.erb partial that defines shareable SVGs for use in this manner:

<svg hidden xmlns="http://www.w3.org/2000/svg">
<symbol id="icon-thumbtack" attributes="from svg">
  <path data />
</symbol>
</svg>

and then referenced like:

<svg><use href="#icon-name"></svg>

These can then be styled with classes as usual, with allowance for using "fill" and "stroke" to color the paths and strokes in.

With a few extra changes, these can also be loaded into the react code and referenced the same way.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice idea, I updated to use the svg this way in rails. Anyway I tried and doesn't find for now a way to use the same in React. I used the svg directly there. Do you have a suggestion for how to make it work?

</div>

<% if story.language.present? %>
<span class="badge badge-red"><%= story.language %></span>
Expand Down
25 changes: 23 additions & 2 deletions rails/app/views/dashboard/stories/edit.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,32 @@
<% end %>

<% content_for(:main_heading) do %>
<h2>
<h2 class="story-main-heading">
<% if @story.topic.present? %>
<div class="small"><%= @story.topic %></div>
<% end %>
<%= @story.title %>
<div class="story-title">
<%= @story.title %>
<%= form_with model: @story, multipart: true, class: "form", data: {remote: false}, locale: true do |f| %>
<% if current_user.admin? || current_user.editor? %>
<% if @story.story_pinned %>
<%= f.hidden_field :story_pinned, value: false %>
<div class="story-pinned">
<%= f.button nil, class: "icon-pinned" do %>
<%= image_tag 'thumbtack-pinned.svg', alt: 'pinned-story', class: "image-pinned" %>
<% end %>
</div>
<% else %>
<%= f.hidden_field :story_pinned, value: true %>
<div class="story-pinned">
<%= f.button nil, class: "icon-not-pinned" do %>
<%= image_tag 'thumbtack-not-pinned.svg', alt: 'not-pinned-story', class: "image-pinned" %>
<% end %>
</div>
<% end %>
<% end %>
<% end %>
</div>
<div class="small">
<% if @story.language.present? %>
<span class="badge"><%= @story.language %></span>
Expand Down
3 changes: 2 additions & 1 deletion rails/app/views/home/_home.json.jbuilder
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
json.stories stories do |story|
json.extract! story, :title, :desc, :id, :created_at
json.extract! story, :title, :desc, :id, :created_at, :story_pinned
json.points story.places.map(&:point_geojson)
json.places story.places
json.language story.language
Expand Down Expand Up @@ -28,6 +28,7 @@ if current_user
end
end
json.logo_path image_path("logocombo.svg")
json.thumbtack_path image_path("thumbtack-pinned.svg")

json.mapbox_access_token @community.theme.mapbox_token
json.mapbox_style @community.theme.mapbox_style
Expand Down
5 changes: 5 additions & 0 deletions rails/db/migrate/20231009012520_add_story_pinned_to_stories.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
class AddStoryPinnedToStories < ActiveRecord::Migration[6.1]
def change
add_column :stories, :story_pinned, :boolean, default: false
end
end
Loading