Redesign: Remove reference to commentable in activity cell by entantoencuanto · Pull Request #10816 · decidim/decidim

entantoencuanto · 2023-05-03T17:22:28Z

🎩 What? Why?

This PR removes the link to commentable present in the title of items of activity feed related with comments creation and replaces it with a generic "New comment:"

📌 Related Issues

Link your PR to an issue

Fixes Redesign: Last activity feed #10370

Testing

Visit https://decidim-redesign.populate.tools/last_activities?filter%5Bwith_resource_type%5D=&filter%5Bwith_resource_type%5D=Decidim%3A%3AComments%3A%3AComment

📷 Screenshots

Please add screenshots of the changes you are proposing

♥️ Thank you!

furilo · 2023-05-05T07:18:39Z

In this case, according to the issue, this should be the proposal where the comment happened, that is, the inmediate "parent" - right @decidim/product ?

https://decidim-redesign.populate.tools/processes/Decidim4Dummies

furilo · 2023-05-08T09:21:24Z

@decidim/product we have been talking about this, we see several cases:

Last Activity at the level site (example):
- Item of component (ie. proposal): it makes sense to show the name of the space
- Comment of Item (ie. a comment in a proposal): if we only show the name of the Item, we would be lacking the name of the space. Alternative: Show a minibreadcrumb: Space / Item
Last Activity inside a space (example)
- Item of component (ie. proposal): omit the second line (since we are already in the space)
- Comment of Item (ie. a comment in a proposal): show the item name

carolromero · 2023-05-08T15:53:11Z

@furilo IMO it makes sense what you say about the last activity within a space:

Regarding the last activity at site level, and comparing the two mockups, we came to the conclusion that it's better to show the previous parent, without adding the breadcrumb.

with breadcrumb:

no breadcrumb:

The main points to go with this option are:

Proposal titles already tend to be quite long, and if we add the title of the space we may need to trim probably both.
It's more consistent with the design of the last activity of a space.
This is the current behavior and no problems have been reported (AFAIK)

furilo · 2023-05-08T15:54:24Z

Perfect. All yours @entantoencuanto

carolromero · 2023-05-08T15:57:33Z

Also, playing with the filters, we find more logical to adapt the design so that the activity itself corresponds to them. Does it make sense to you as well?

…ities

…participatory spaces

entantoencuanto · 2023-05-09T10:19:51Z

Changes added, including the first icon previous to the activity title

furilo · 2023-05-11T07:50:44Z

@entantoencuanto why its not showing 10 items? https://decidim-redesign.populate.tools/last_activities (neither in the second page)

Also, @decidim/product, should we change the default items per page to 20?

carolromero

Hi @entantoencuanto, to finish with the review of this PR:

use the same font color for all activity content (right now the first part has a black color that stands out too much).
remove the participation space type icon
Regarding the activity of a space:

Rename the content block to "Last Activity" and remove the subtitle
Missing link (and page) to see all the activity of the space
Unify font styles

@furilo I would say that 10 items by default is fine but yes, it seems that right now the filter is not working properly. FWIW even if you set the content block to 10, only 6 items are displayed.

entantoencuanto · 2023-05-18T14:42:39Z

@entantoencuanto why its not showing 10 items? https://decidim-redesign.populate.tools/last_activities (neither in the second page)

Also, @decidim/product, should we change the default items per page to 20?

Taking a look to the code the paginated results are filtered to display only visible items:

From the controller there is a query which generates a paginated list of activities
From the view there is a call to a cell with the list of activities. The cell filters the passed list to show only activities visible by current user

To fix this the visibility scope should be passed to the query before paginate it, but I'm not sure how complex this would be.

furilo · 2023-05-19T10:05:00Z

@carolromero it seems the malfunctioning of the item list is a current problem, I opened an issue: #10882

…-comments * feature/redesign: Add redesign enable ENV variable to control the pipeline (#10610) Redesign: progress bar (#10638) Redesign: meeting cards (#10722) Redesign: pending login (#10699) simplify 2col layout (#10819)

alecslupu

Proper visuals encountered

decidim-core/app/cells/decidim/content_blocks/participatory_space_last_activity/content.erb

* feature/redesign: Redesign: debates (#10653) Redesign: Remove reference to commentable in activity cell (#10816) Redesign: spinner (#10848) Redesign: omnipresent banner (#10847)

…rafts * feature/redesign: Feature/redesign components breadcrumb (#10441) Redesign: debates (#10653) Redesign: Remove reference to commentable in activity cell (#10816) Redesign: spinner (#10848) Redesign: omnipresent banner (#10847) Add redesign enable ENV variable to control the pipeline (#10610) Redesign: progress bar (#10638) Redesign: meeting cards (#10722) Redesign: pending login (#10699) simplify 2col layout (#10819) Redesign: processes groups content blocks (#10491) Redesign: assemblies content blocks (#10573)

* feature/redesign: (21 commits) Feature/redesign components breadcrumb (#10441) Redesign: debates (#10653) Redesign: Remove reference to commentable in activity cell (#10816) Redesign: spinner (#10848) Redesign: omnipresent banner (#10847) Add redesign enable ENV variable to control the pipeline (#10610) Redesign: progress bar (#10638) Redesign: meeting cards (#10722) Redesign: pending login (#10699) simplify 2col layout (#10819) Redesign: processes groups content blocks (#10491) Redesign: assemblies content blocks (#10573) Redesign: process & process group cards (#10716) Redesign: conference cards (#10502) Redesign: pending blogs (#10686) Redesign: blog cards (#10685) Redesign: filters (#10390) replace uses of specific margin-bottom for layout margins (#10675) Redesign: menu mobile (#10351) Fix comments scss to avoid compilation errors (#10657) ...

* feature/redesign: Feature/redesign components breadcrumb (#10441) Redesign: debates (#10653) Redesign: Remove reference to commentable in activity cell (#10816) Redesign: spinner (#10848) Redesign: omnipresent banner (#10847)

…blies-details-page * feature/redesign: Redesign: amendments (#10765) Redesign: proposals (#10555) Remove unused preset-env dependencies (#10916) Feature/redesign components breadcrumb (#10441) Redesign: debates (#10653) Redesign: Remove reference to commentable in activity cell (#10816) Redesign: spinner (#10848) Redesign: omnipresent banner (#10847) Add redesign enable ENV variable to control the pipeline (#10610)

* feature/redesign: Redesign: amendments (#10765) Redesign: proposals (#10555) Remove unused preset-env dependencies (#10916) Feature/redesign components breadcrumb (#10441) Redesign: debates (#10653) Redesign: Remove reference to commentable in activity cell (#10816) Redesign: spinner (#10848) Redesign: omnipresent banner (#10847) Add redesign enable ENV variable to control the pipeline (#10610)

* feature/redesign: Redesign: amendments (#10765) Redesign: proposals (#10555) Remove unused preset-env dependencies (#10916) Feature/redesign components breadcrumb (#10441) Redesign: debates (#10653) Redesign: Remove reference to commentable in activity cell (#10816) Redesign: spinner (#10848) Redesign: omnipresent banner (#10847) Add redesign enable ENV variable to control the pipeline (#10610) Redesign: progress bar (#10638) Redesign: meeting cards (#10722) Redesign: pending login (#10699)

entantoencuanto added the project: redesign Barcelona City Council contract label May 3, 2023

entantoencuanto marked this pull request as ready for review May 3, 2023 18:21

entantoencuanto requested review from ferblape and furilo May 3, 2023 18:22

entantoencuanto added 2 commits May 4, 2023 15:38

Remove reference to commentable in activity cell

25381ce

Fix test

a4d230e

entantoencuanto force-pushed the feature/redesign-last-activities-comments branch from 7f4ae36 to a4d230e Compare May 4, 2023 13:38

furilo approved these changes May 5, 2023

View reviewed changes

furilo self-requested a review May 5, 2023 07:18

entantoencuanto added 6 commits May 8, 2023 20:06

Move participatory space icon in activity cells to a method

c8b7674

Use root commentable instead of participatory space in comments activ…

ae3306b

…ities

Add resource type icon for each activity

8c6eba9

Hide participatory space in last activities inside content blocks of …

6b95d9c

…participatory spaces

Force not hidding participatory space on comments activity cell

1dd1cdf

Add styles to activity title icon

407408a

ferblape approved these changes May 11, 2023

View reviewed changes

furilo requested a review from a team May 11, 2023 15:53

carolromero requested changes May 15, 2023

View reviewed changes

entantoencuanto and others added 2 commits May 18, 2023 17:16

Change title and remove subtitle of last activities content block

ef7a05e

remove icon and add text-gray to first span

f4500e1

furilo mentioned this pull request May 19, 2023

Last Activity page: not all items are being shown #10882

Open

furilo requested a review from carolromero May 19, 2023 10:11

furilo approved these changes May 19, 2023

View reviewed changes

carolromero approved these changes May 22, 2023

View reviewed changes

alecslupu approved these changes May 22, 2023

View reviewed changes

decidim-core/app/cells/decidim/content_blocks/participatory_space_last_activity/content.erb Outdated Show resolved Hide resolved

decidim-core/app/cells/decidim/content_blocks/participatory_space_last_activity/content.erb Outdated Show resolved Hide resolved

Remove unnecessary comments

83d2ce3

ferblape merged commit 5a93c81 into feature/redesign May 23, 2023

ferblape deleted the feature/redesign-last-activities-comments branch May 23, 2023 02:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Redesign: Remove reference to commentable in activity cell#10816

Redesign: Remove reference to commentable in activity cell#10816
ferblape merged 12 commits intofeature/redesignfrom
feature/redesign-last-activities-comments

entantoencuanto commented May 3, 2023

Uh oh!

furilo commented May 5, 2023

Uh oh!

furilo commented May 8, 2023

Uh oh!

carolromero commented May 8, 2023

Uh oh!

furilo commented May 8, 2023

Uh oh!

carolromero commented May 8, 2023

Uh oh!

entantoencuanto commented May 9, 2023

Uh oh!

furilo commented May 11, 2023

Uh oh!

carolromero left a comment •

edited by furilo

Loading

Uh oh!

entantoencuanto commented May 18, 2023

Uh oh!

furilo commented May 19, 2023

Uh oh!

alecslupu left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Uh oh!

Conversation

entantoencuanto commented May 3, 2023

🎩 What? Why?

📌 Related Issues

Testing

📷 Screenshots

Uh oh!

furilo commented May 5, 2023

Uh oh!

furilo commented May 8, 2023

Uh oh!

carolromero commented May 8, 2023

Uh oh!

furilo commented May 8, 2023

Uh oh!

carolromero commented May 8, 2023

Uh oh!

entantoencuanto commented May 9, 2023

Uh oh!

furilo commented May 11, 2023

Uh oh!

carolromero left a comment • edited by furilo Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

entantoencuanto commented May 18, 2023

Uh oh!

furilo commented May 19, 2023

Uh oh!

alecslupu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

carolromero left a comment •

edited by furilo

Loading