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

Ticket/4464 imageless cards #4470

Merged
merged 1 commit into from
Oct 29, 2024
Merged

Conversation

dev-rana-publicis
Copy link
Contributor

@dev-rana-publicis dev-rana-publicis commented Oct 15, 2024

Closes #4464

Copy link
Member

@bryanpizzillo bryanpizzillo left a comment

Choose a reason for hiding this comment

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

@dev-rana-publicis can you please put the imageless cards into cgov_promo with the feature cards? The row can stay in home landing like they do now.

@dev-rana-publicis dev-rana-publicis force-pushed the ticket/4464-imageless-cards branch from a2147f4 to 594cebc Compare October 16, 2024 20:56
@dlescarbeau dlescarbeau force-pushed the ticket/4464-imageless-cards branch 4 times, most recently from c719ae2 to ac0e642 Compare October 23, 2024 03:49
@dlescarbeau dlescarbeau marked this pull request as ready for review October 23, 2024 14:02
@dlescarbeau dlescarbeau force-pushed the ticket/4464-imageless-cards branch 2 times, most recently from 6cbcb94 to 55546c4 Compare October 23, 2024 14:13
@cgdp-management-server
Copy link

ODE Deployment

Code has been deployed to ODE 848.

@welshja
Copy link
Collaborator

welshja commented Oct 23, 2024

@dlescarbeau - Everything is working from an analytics perspective as defined on the ticket, but seeing a change that had been discussed, though didn't get translated onto the github ticket. This is the change we'd like to make with the analytics:

componentVariant: This should be set based on the layout selected by the user. This should be set to 1 Card Row, 2 Card Row or 3 Card Row.

Currently the componentVariant value is set to none.

Sorry for the mixup on this.

@KateMashkinaNIH
Copy link

  1. Machine name should be replaced by "Card Layout Display" per AC and value to default to "3 colums"
    Screenshot 2024-10-23 at 1 57 06 PM
  2. Edit option should not be there for Internal Card
Screenshot 2024-10-23 at 2 15 26 PM 3. Cards width is different in the manually created MLP Screenshot 2024-10-23 at 2 37 59 PM

@monika-jaeger
Copy link

The spacing below the card title is still on the imageless and defaults cards when there is no description.
I don’t see we have comps defined for default & imageless cards with a title only, but we do for flag cards, so we’d want all cards to be consistent. (this might be a separate CR to address all)

@adrianacastaneda
Copy link

adrianacastaneda commented Oct 23, 2024

@dlescarbeau Label for the card display field should be "Card Layout Display" and it should be above the Heading field. Thank you!

Updated Decision: This will not be changed for consistency with List.

@adrianacastaneda
Copy link

adrianacastaneda commented Oct 23, 2024

Also, noticed an "Edit" button on the featured item field that has a weird experience. It leads to editing the fields of the actual page. This is a different user experience than we usually support for card components so I don't think we should move forward it. Having the "remove" button should be enough!

Screenshot 2024-10-23 at 3 02 08 PM
Screenshot 2024-10-23 at 3 01 54 PM

@bryanpizzillo
Copy link
Member

Also, noticed an "Edit" button on the featured item field that has a weird experience. It leads to editing the fields of the actual page. This is a different user experience than we usually support for card components so I don't think we should move forward it. Having the "remove" button should be enough!

That is the paragraphs setting. I will just add it to my ever growing list. 😃

@dlescarbeau dlescarbeau force-pushed the ticket/4464-imageless-cards branch 2 times, most recently from 312e613 to cd461f7 Compare October 23, 2024 20:16
@andyvanavery31
Copy link

andyvanavery31 commented Oct 23, 2024

@dlescarbeau can you please fix:

Front-end:

  • The spacing above the optional group heading is wrong and got missed on the storybook testing and that messes up our section spacing rules. It currently has the -16px in the padding of the section, but the additional 19.920px that got brought over from storybook to make the stories work on one page is adding additional whitespace. The total space between the component above it and the top of the group heading should be 32px.

Drupal:

  • One imageless card does not automatically get added to the paragraph when the group gets added to the page
  • For the cards themselves, the Override Title, Override Description (Internal & Multimedia), Card Title, and Card Description (External) fields do not have the tooltip text from the ACs
  • You should not be able to edit the contents of the feature item on internal cards. You should only be able to select and replace the item.
  • Change the field name to field_imgless_crd_grp_display
  • The heading looks to be Open Sans, not Poppins like the other MLP card elements.
  • In the Drupal form, when there is more than one card, they should collapse. See 1st screenshot below where I go back to edit a page and the Imageless Cards are not collapsed but do have the ability to collapse if the user selects it. The 2nd screenshot demonstrates how the feature cards are automatically collapsed. The imageless cards should act the same way.
  • See Kate's 2 unit tests comments

@dlescarbeau dlescarbeau force-pushed the ticket/4464-imageless-cards branch 2 times, most recently from 734224f to 7cb7325 Compare October 24, 2024 14:32
@lburack
Copy link

lburack commented Oct 25, 2024

  • The heading looks to be Open Sans, not Poppins like the other MLP card elements.
  • In the Drupal form, when there is more than one card, they should collapse. See 1st screenshot below where I go back to edit a page and the Imageless Cards are not collapsed but do have the ability to collapse if the user selects it. The 2nd screenshot demonstrates how the feature cards are automatically collapsed. The imageless cards should act the same way.
    Screenshot 2024-10-25 at 1 00 10 PM

Screenshot 2024-10-25 at 1 00 17 PM

@dlescarbeau dlescarbeau force-pushed the ticket/4464-imageless-cards branch from 0508e10 to 619231a Compare October 25, 2024 18:26
@KateMashkinaNIH
Copy link

external card URL field should be a simple input - it should not allow an internal link search
Screenshot 2024-10-25 at 4 58 39 PM

@bryanpizzillo
Copy link
Member

@andyvanavery31 and @lburack -

In the Drupal form, when there is more than one card, they should collapse. See 1st screenshot below where I go back to edit a page and the Imageless Cards are not collapsed but do have the ability to collapse if the user selects it. The 2nd screenshot demonstrates how the feature cards are automatically collapsed. The imageless cards should act the same way.

I am going to make this be the same configuration as Feature Cards as you "ask". However, The Feature Card Row does not change its behavior on the number -- field_row_cards is always collapsed once it is saved. So remove "when there is more than one card, they should collapse." from the ask.

Also it should be noted that the flag card group does not follow this behavior.

@bryanpizzillo bryanpizzillo force-pushed the ticket/4464-imageless-cards branch from 619231a to a6eadb9 Compare October 25, 2024 21:56
@bryanpizzillo bryanpizzillo force-pushed the ticket/4464-imageless-cards branch from a6eadb9 to 6f0b13f Compare October 28, 2024 15:21
@KateMashkinaNIH KateMashkinaNIH added the Passed QA review Regression testing found no issues. label Oct 28, 2024
@bennettcc
Copy link

This PR has been approved by the Federal team as of 10/29 - Alex and Anna have reviewed

Copy link

@andyvanavery31 andyvanavery31 left a comment

Choose a reason for hiding this comment

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

Passes Product Review.

@bryanpizzillo bryanpizzillo merged commit 13ec98e into develop Oct 29, 2024
5 checks passed
@bryanpizzillo bryanpizzillo deleted the ticket/4464-imageless-cards branch October 29, 2024 13:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Passed QA review Regression testing found no issues.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Story: Create Imageless Card Group Paragraph for MLP
10 participants