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.

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

Issue 2084/activity list items layout - adding subtitles #2213

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

neta-kedem
Copy link
Collaborator

Description

This PR adds subtitles to activity list items, and makes minor UI changes to show it

Screenshots

Screenshot 2024-09-29 at 13 10 18

Changes

  • Adds subtitles to activity list items

Notes to reviewer

not fully polished, but it's working so I'm pushing for the demo

Related issues

Resolves #2084

Copy link
Member

@richardolsson richardolsson left a comment

Choose a reason for hiding this comment

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

Nice work @neta-kedem!

I'll take a closer look at the code, but the first thing I noticed when I tried the preview build is that the stats have moved out of alignment.

This is what an activity list looks like on main (running on app.dev.zetkin.org):

image

Below is what it looks like in the preview build for this branch. Note how the little numeric chips are no longer aligned along the same vertical line.

image

Can you take a look at this before a more thorough review of the code?

@ziggabyte
Copy link
Contributor

Hi @neta-kedem ! I see that you made an update to the pr a while back, sorry for the slow response from us! When i look at the deployed version the alignment is still wonky like Richard commented. Do you wanna look into it again?

@neta-kedem
Copy link
Collaborator Author

Yes, I'll look back into it tomorrow at the hackathon

# Conflicts:
#	src/features/campaigns/components/ActivityList/items/CanvassAssignmentListItem.tsx
Copy link
Contributor

@ziggabyte ziggabyte left a comment

Choose a reason for hiding this comment

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

Woop woop looking good! Looking at the build it looks very neat and i like getting the information about the deadlines/start dates in the activity overview like this!

However, we do have an issue - it's not realted to code that you have written, directly, but it's the ZUIRelativeTime component that is not behaving very well when it comes to indicating things happening "tomorrow":

bild
bild
bild

As you can see, the ZUIRelativeTime says that this task has its deadline "tomorrow" - which is not true, it has a deadline on December 12 and the day I am viewing this is December 10. We have recently have had another volunteer attempt to attack this problem, without a concrete solution. I'm gonna loop in @richardolsson here to see what his thoughts are.

My opinion is that it's problematic to have a component give misleading information like this, but I also feel like it's a bit of a can of worms to try to solve the underlying issue...

So this is why I'm marking this as "changes requested" for the moment, even though I cannot at this exact moment say what specific changes should be made to make this a more truthful component....

@@ -65,19 +67,21 @@ export enum STATUS_COLORS {
RED = 'red',
}

export type AcitivityListItemProps = {
Copy link
Contributor

Choose a reason for hiding this comment

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

🏅 ⭐ for finding one of these miss-spellings!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update narrow activity list items layout
3 participants