[RFC] Use cells for meeting m cards by agustibr · Pull Request #3022 · decidim/decidim

agustibr · 2018-03-16T13:21:15Z

🎩 What? Why?

This PR starts from commit ea666c7 from @mrcasals and is possible to merge with PR #2897 that includes proposal_m cell.

This PR is done with the idea to make the code review easier.

Adding card_for helper in decidim-core to facilitate te use of cards from everywhere, the Component should have the attribute card pointing to the cell's component path, making it possible (size is optional):

<%= card_for my_meeting, size: :m %>

Adding the meeting_cell component to decidim-meetings (decidim-core already has cells from @decidim/lot-core) in two sizes or variations; medium and list_item, the following will render the medium size:

<%= cell "decidim/meetings/meeting", my_meeting %>
<%= cell "decidim/meetings/meeting", my_meeting, size: :m %>
<%= cell "decidim/meetings/meeting_m", my_meeting %>

The medium cell includes a subcell header:

# when used *inside* the `meeting_m` cell
<%= render :head %>

# when used *outside* the `meeting_m` cell
<%= cell("decidim/meetings/meeting", my_meeting).head %>

he following will render the list_iteml size:

<%= cell "decidim/meetings/meeting", meeting, size: :list_item %>

📌 Related Issues

Related to Introduce cells to componentize the UI #2521
Related to [RFC] Use cells to componentize the views (2nd take) #2897

📋 Subtasks

📷 Screenshots (optional)

meeting :m card:

meeting :h card:

mrcasals · 2018-03-16T13:27:52Z

decidim-meetings/app/cells/decidim/meetings/meeting_m/show.erb

+<div class="column">
+  <article class="card card--meeting">
+    <div class="card__content">
+      <%= cell("decidim/meetings/meeting_m", model).header %>


<%= render :header %>, I think

I've tried it, it also works, do you prefer <%= render :header %>?

codecov · 2018-03-19T16:41:22Z

Codecov Report

Merging #3022 into master will decrease coverage by 0.02%.
The diff coverage is 90.64%.

@@            Coverage Diff             @@
##           master    #3022      +/-   ##
==========================================
- Coverage   98.62%   98.59%   -0.03%     
==========================================
  Files        1699     1711      +12     
  Lines       40602    40788     +186     
==========================================
+ Hits        40042    40216     +174     
- Misses        560      572      +12

mrcasals · 2018-03-20T13:11:08Z

@agustibr there are some conflicts 😄

mrcasals · 2018-03-20T13:21:51Z

decidim-meetings/spec/cells/decidim/meetings/meeting_cell_spec.rb

+    let!(:meeting) { create(:meeting) }
+
+    context "when rendering" do
+      it do


it "renders the card"?

Ok, i'll add the description

mrcasals · 2018-03-20T13:21:56Z

decidim-meetings/spec/cells/decidim/meetings/meeting_list_item_cell_spec.rb

+    let!(:meeting) { create(:meeting) }
+
+    context "when rendering" do
+      it do


it "renders the card"?

mrcasals · 2018-03-20T13:22:00Z

decidim-meetings/spec/cells/decidim/meetings/meeting_m_cell_spec.rb

+    let!(:meeting) { create(:meeting) }
+
+    context "when rendering" do
+      it do


it "renders the card"?

agustibr · 2018-03-20T16:37:24Z

@mrcasals I've fixed the merge conflict

oriolgual

Nice work @agustibr! Just some minor things.

oriolgual · 2018-03-21T08:37:16Z

decidim-core/app/cells/decidim/author_box_cell.rb

+
+    delegate :user_signed_in?, to: :parent_controller
+
+    delegate :current_user, to: :parent_controller


Could you please merge this two delegates?

oriolgual · 2018-03-21T08:38:37Z

decidim-core/app/cells/decidim/card_cell.rb

+    end
+
+    def resource_cell
+      model.component.manifest.card


Maybe memoize this?

@resource_cell ||= model.component.manifest.card

oriolgual · 2018-03-21T08:39:02Z

decidim-core/app/cells/decidim/card_cell.rb

+
+module Decidim
+  class CardCell < Decidim::ViewModel
+    # This cell renders the card of the given instance of a Component


Could you please move the docs above the class?

oriolgual · 2018-03-21T08:39:21Z

decidim-core/app/cells/decidim/profile_cell.rb

+
+    delegate :user_signed_in?, to: :parent_controller
+
+    delegate :current_user, to: :parent_controller


Can you also merge these?

oriolgual · 2018-03-21T08:39:56Z

decidim-meetings/app/cells/decidim/meetings/meeting_cell.rb

+  module Meetings
+    class MeetingCell < Decidim::Meetings::ViewModel
+      # This cell renders the meeting card for an instance of a Meeting
+      # the default size is the Medium Card (:m)


Can you move the docs above the class?

oriolgual · 2018-03-21T08:40:35Z

decidim-meetings/app/cells/decidim/meetings/meeting_list_item_cell.rb

+module Decidim
+  module Meetings
+    class MeetingListItemCell < Decidim::Meetings::MeetingCell
+      # This cell renders the List Item Card (:list_item) meeting card


Can you move the docs above the class?

oriolgual · 2018-03-21T08:40:42Z

decidim-meetings/app/cells/decidim/meetings/meeting_m_cell.rb

+module Decidim
+  module Meetings
+    class MeetingMCell < Decidim::Meetings::MeetingCell
+      # This cell renders the Medium (:m) meeting card


Can you move the docs above the class?

agustibr · 2018-03-21T10:26:04Z

@oriolgual thanks, I have updated/reviewed the PR considering your comments.

mrcasals · 2018-03-21T11:23:28Z

OMG 😍 🎉 🎈 🎊

* master: [RFC] Use cells for meeting m cards (#3022) Do not force Postgresql user to be admin when enabling trigram extension (#3053) Make organization reference_prefix required (#3056) admin can duplicate/copy meetings (#3051) Fix question form errors not being displayed (#3046) Erb whitespace cutting (#3047) Show debates statistics on space show and homepage (#3016) Fix broken translated field after form errors (#3026) Move decidim executable to "exe" folder (#3028) Friendlier buttons (#3027) Feedback needed after Endorsing when user has no user_groups (#2998) Fix seeding error on generator specs (#3021) fix spelling error in threshold (#3019) Migration plus seeds (#2933)

agustibr · 2018-03-21T13:54:22Z

YAY! 🏄‍♂️ 🎉
I'll go for #2897 🏇

josepjaume and others added 12 commits March 3, 2018 15:58

Introduce cells to componentize the UI

f17dddf

Fix layout

548b9b4

Update gems

ee5012f

Fix layout

534e560

Improve naming

6ba007a

Add UserCell

c179db2

Render endorsers using cells

b380ff5

Fix layout

9ba756a

Improve naming and reuse cells

ca23bf5

Reuse AuthorBox cell

ea666c7

Merge branch 'master' into cells-reloaded-meetings

5c4311d

[RFC] Introduce Meeting card cells, size :m and :h

8eb8a76

ghost assigned agustibr Mar 16, 2018

ghost added the status: WIP label Mar 16, 2018

agustibr force-pushed the cells-reloaded-meetings branch from 3801b85 to 8eb8a76 Compare March 16, 2018 13:21

mrcasals reviewed Mar 16, 2018

View reviewed changes

tramuntanal mentioned this pull request Mar 19, 2018

[WIP] decidim-search MVP #2583

Closed

8 tasks

agustibr added 3 commits March 19, 2018 17:30

Merge branch 'master' into cells-reloaded-meetings

a1e11b2

remove unused show for meeting cell

b20e985

change to render :header

d85ec53

fixed translation missing: en.decidim.profile.deleted

2f87d55

agustibr force-pushed the cells-reloaded-meetings branch 2 times, most recently from 3b05d3d to a3a526d Compare March 20, 2018 09:01

fix cells/author

576a6f7

agustibr force-pushed the cells-reloaded-meetings branch from a3a526d to 576a6f7 Compare March 20, 2018 09:01

agustibr added in-review and removed status: WIP labels Mar 20, 2018

Renamed cell :h to :list_item added docs to meeting cells

da45d9e

ghost added the status: WIP label Mar 20, 2018

added meeting cell specs

68a74db

agustibr force-pushed the cells-reloaded-meetings branch from da05d74 to 68a74db Compare March 20, 2018 13:04

mrcasals reviewed Mar 20, 2018

View reviewed changes

agustibr added 3 commits March 20, 2018 14:53

added description to it

965669c

Fix dependencies should be sorted in an alphabetical

9e42599

Merge branch 'master' into cells-reloaded-meetings

69898a9

agustibr removed the status: WIP label Mar 20, 2018

agustibr mentioned this pull request Mar 20, 2018

Tags & mentions #2684

Merged

2 tasks

mrcasals requested review from josepjaume and oriolgual March 21, 2018 08:22

oriolgual suggested changes Mar 21, 2018

View reviewed changes

agustibr added 4 commits March 21, 2018 10:53

fix, merge the delegates in oneliner

271969c

fix, move the docs above the class

f0776d6

memoize resource_cell

06e5443

Add CHANGELOG entries

dfc4964

ghost added the status: WIP label Mar 21, 2018

agustibr removed the status: WIP label Mar 21, 2018

oriolgual approved these changes Mar 21, 2018

View reviewed changes

oriolgual merged commit 89b050d into master Mar 21, 2018

oriolgual deleted the cells-reloaded-meetings branch March 21, 2018 11:22

deivid-rodriguez mentioned this pull request Mar 21, 2018

Codeclimate PR check shows as green when actually failing #3068

Closed

agustibr removed the in-review label Apr 17, 2018

snyk-bot mentioned this pull request Jan 12, 2022

[Snyk] Security upgrade axios from 0.18.1 to 0.20.0 OpenSourcePolitics/decidim#1169

Closed

snyk-bot mentioned this pull request Feb 10, 2022

[Snyk] Security upgrade axios from 0.18.1 to 0.20.0 OpenSourcePolitics/decidim#1174

Closed


		delegate :user_signed_in?, to: :parent_controller

		delegate :current_user, to: :parent_controller

Uh oh!

Conversation

agustibr commented Mar 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🎩 What? Why?

📌 Related Issues

📋 Subtasks

📷 Screenshots (optional)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

agustibr Mar 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Mar 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mrcasals commented Mar 20, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

agustibr commented Mar 20, 2018

Uh oh!

oriolgual left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

agustibr commented Mar 21, 2018

Uh oh!

mrcasals commented Mar 21, 2018

Uh oh!

agustibr commented Mar 21, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

agustibr commented Mar 16, 2018 •

edited

Loading

agustibr Mar 16, 2018 •

edited

Loading

codecov bot commented Mar 19, 2018 •

edited

Loading