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

feat: added F14 base category questions #1738

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

Conversation

nathanbogale
Copy link
Contributor

Description

This pull request introduces a reorganization of the proposal schema to enhance clarity and ensure proper categorization of proposals. The key changes are as follows:

Changes Made

  • Added a new segment titled "Campaign Category" to better organize proposal eligibility criteria.
  • Moved the "Category Questions" under the new "Campaign Category" segment.
  • Each question now references the yesNoChoice type from the schema definitions to standardize responses.
  • Each question within the "Category Questions" section includes a guidance card and a brief citation to provide context and clarity for proposers.

Please confirm the following checks

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream module

@nathanbogale nathanbogale added review me PR is ready for review comment me Request for comments F14 labels Jan 30, 2025
@nathanbogale nathanbogale added this to the M4: Voting & Delegation milestone Jan 30, 2025
@nathanbogale nathanbogale self-assigned this Jan 30, 2025
Copy link
Contributor

Test Report | ${\color{lightgreen}Pass: 455/455}$ | ${\color{red}Fail: 0/455}$ |

Copy link
Contributor

Test Report | ${\color{lightgreen}Pass: 455/455}$ | ${\color{red}Fail: 0/455}$ |

Copy link
Contributor

Test Report | ${\color{lightgreen}Pass: 455/455}$ | ${\color{red}Fail: 0/455}$ |

Copy link
Contributor

Test Report | ${\color{lightgreen}Pass: 455/455}$ | ${\color{red}Fail: 0/455}$ |

stevenj
stevenj previously approved these changes Feb 3, 2025
Copy link
Collaborator

@stevenj stevenj left a comment

Choose a reason for hiding this comment

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

Lgtm

Copy link
Contributor

github-actions bot commented Feb 3, 2025

Test Report | ${\color{lightgreen}Pass: 457/457}$ | ${\color{red}Fail: 0/457}$ |

@dtscalac
Copy link
Contributor

dtscalac commented Feb 4, 2025

@nathanbogale
The schema has to follow certain structure so that menu entries and topics are shown correctly. The pattern is:
segment -> section -> topic -> [topic].

If there's no segment -> section then frontend simply ignores this "branch" and doesn't display it. I've added suggestions in two places what needs to be changed. With current schema the campaign_category is not shown at all because segment -> section pattern is not present.

@nathanbogale
Copy link
Contributor Author

@dtscalac
Now and in ffa7d13 the category field strictly follows a segment -> section structure.

  • campaign_category is set as the segment while
  • category_questions is set as the section that will have all the topics under it defined as type objects

Copy link
Contributor

github-actions bot commented Feb 4, 2025

Test Report | ${\color{lightgreen}Pass: 457/457}$ | ${\color{red}Fail: 0/457}$ |

@nathanbogale nathanbogale requested a review from dtscalac February 4, 2025 10:37
dtscalac
dtscalac previously approved these changes Feb 4, 2025
Copy link
Contributor

@dtscalac dtscalac left a comment

Choose a reason for hiding this comment

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

Looks ok

Screenshot 2025-02-04 at 12 45 14

@dtscalac
Copy link
Contributor

dtscalac commented Feb 4, 2025

@nathanbogale
Optionally, the "title": "If Yes", could be on the question level, not on the parent level. For topics we only show the title if this title is set for the topic, not for it's parent.

The description could be changed to x-placeholder if you'd like to show it as hint inside the text field, like so:

"x-placeholder": "What is the name of the enterprise or team, and what is their track record in the industry?"

Copy link
Contributor

github-actions bot commented Feb 4, 2025

Test Report | ${\color{lightgreen}Pass: 464/464}$ | ${\color{red}Fail: 0/464}$ |

@nathanbogale
Copy link
Contributor Author

@dtscalac
the If Yes is in the parent level so its more scalable and we can accommodate additional types and topics under it as per need, since this is the early stages of categories in proposals.
as for the x-guidance, as far as the document goes, the provided x-guidance is shared between the parent and the if-yes level in topics, i put it in the parent section because technically it can cover the child topic as well.. and the description is more of a heading to the textbox under if yes, that it might even be better to change it to a title

Copy link
Contributor

github-actions bot commented Feb 4, 2025

Test Report | ${\color{lightgreen}Pass: 464/464}$ | ${\color{red}Fail: 0/464}$ |

@dtscalac
Copy link
Contributor

dtscalac commented Feb 4, 2025

@dtscalac the If Yes is in the parent level so its more scalable and we can accommodate additional types and topics under it as per need, since this is the early stages of categories in proposals. as for the x-guidance, as far as the document goes, the provided x-guidance is shared between the parent and the if-yes level in topics, i put it in the parent section because technically it can cover the child topic as well.. and the description is more of a heading to the textbox under if yes, that it might even be better to change it to a title

Okay, I'll handle the parent's title as well. Please note that if both the parent and the child have titles there would be two titles, one under the other.

@dtscalac
Copy link
Contributor

dtscalac commented Feb 4, 2025

@nathanbogale Here's a screenshot after handling the parent's title:

Screenshot 2025-02-04 at 14 12 27

@nathanbogale
Copy link
Contributor Author

@dtscalac
we can change the descriptions in the if yes text field into x-placeholders, that way we avoid having two titles, and use the same x-guidance for both parent and child, and it will generally display better

Copy link
Contributor

github-actions bot commented Feb 5, 2025

Test Report | ${\color{lightgreen}Pass: 467/467}$ | ${\color{red}Fail: 0/467}$ |

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comment me Request for comments F14 review me PR is ready for review
Projects
Status: 🔖 Ready
Development

Successfully merging this pull request may close these issues.

3 participants