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

Add optional order for initiative content block #7047

Merged
merged 6 commits into from
Feb 8, 2021
Merged

Add optional order for initiative content block #7047

merged 6 commits into from
Feb 8, 2021

Conversation

armandfardeau
Copy link
Contributor

@armandfardeau armandfardeau commented Dec 18, 2020

🎩 What? Why?

This feature modify the edit page of the initiative homepage content block, as well as the order of the initiatives displayed in FE on the homepage.

📌 Related Issues

Testing

Create an initiative Highlighted initiatives homepage content block, select order element by most recent.

📋 Checklist

🚨 Please review the guidelines for contributing to this repository.

  • CONSIDER adding a unit test if your PR resolves an issue.
  • ✔️ DO check open PR's to avoid duplicates.
  • ✔️ DO keep pull requests small so they can be easily reviewed.
  • ✔️ DO build locally before pushing.
  • ✔️ DO make sure tests pass.
  • ✔️ DO make sure any new changes are documented in docs/.
  • ✔️ DO add and modify seeds if necessary.
  • ✔️ DO add CHANGELOG upgrade notes if required.
  • ✔️ DO add to GraphQL API if there are new public fields.
  • ✔️ DO add link to MetaDecidim if it's a new feature.
  • AVOID breaking the continuous integration build.
  • AVOID making significant changes to the overall architecture.

📷 Screenshots

Capture d’écran 2020-12-18 à 14 21 28

@armandfardeau
Copy link
Contributor Author

I think the survey tests failure is flaky, could you reload it?

@mrcasals
Copy link
Contributor

@armandfardeau I restarted the job!

@armandfardeau
Copy link
Contributor Author

@armandfardeau I restarted the job!

Thanks @mrcasals

@armandfardeau armandfardeau marked this pull request as ready for review December 18, 2020 16:25
@mrcasals
Copy link
Contributor

@armandfardeau while we review the code, I suspect @decidim/product will want to check it out, can you push this somewhere so they can test it out?

@andreslucena andreslucena changed the title Add optionnal order for initiative content block Add optional order for initiative content block Jan 18, 2021
@armandfardeau
Copy link
Contributor Author

@armandfardeau while we review the code, I suspect @decidim/product will want to check it out, can you push this somewhere so they can test it out?

Sure: https://dcd-order-initiative-content-block.osp.dev/

@mrcasals
Copy link
Contributor

@decidim/product can you review this, please? 😄

@carolromero
Copy link
Member

@armandfardeau quick question: I see one of the sorting options says 'Default (unordered)'. Does this mean 'random' order? If so, can we change the string from 'unordered' to 'random'? Thanks!

@armandfardeau
Copy link
Contributor Author

@armandfardeau quick question: I see one of the sorting options says 'Default (unordered)'. Does this mean 'random' order? If so, can we change the string from 'unordered' to 'random'? Thanks!

The default order returns items based on an active record query. It may appear to the human eye as random but cannot be qualified as such. This is why we preferred the term unordered.

@paulinebessoles
Copy link
Contributor

@carolromero Do you still want us to change the sorting option name or can this PR be merged?
Thanks for your answer!

@carolromero
Copy link
Member

@paulinebessoles @armandfardeau we didn't know about this default order "Unordered" and it doesn't make any sense, can you guys please change it so that the default order is by date of creation? 🙏

@paulinebessoles
Copy link
Contributor

No problem @carolromero , we'll swap the two display options to make the default=most recent!

@armandfardeau
Copy link
Contributor Author

@mrcasals Hi, everything is in order here, tell me if you need anything.

Copy link
Contributor

@mrcasals mrcasals left a comment

Choose a reason for hiding this comment

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

Looks good to me!

Copy link
Contributor

@mrcasals mrcasals left a comment

Choose a reason for hiding this comment

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

All good!

@mrcasals mrcasals merged commit 1f6cdc7 into decidim:develop Feb 8, 2021
@armandfardeau armandfardeau deleted the feature/order-initiative-content-block branch February 8, 2021 09:53
@mrcasals mrcasals added module: initiatives type: feature PRs or issues that implement a new feature labels Feb 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module: initiatives type: feature PRs or issues that implement a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants