-
Notifications
You must be signed in to change notification settings - Fork 4
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
Added a New Card Filter For Labels #9
base: master
Are you sure you want to change the base?
Conversation
Hi @peace-d Thank you for taking the time to submit this feature! We really do appreciate it. I will be giving comments inline as well as on the your comment on the related issue but here is some general feedback: Firstly, in terms of the feature that is discussed in #2, we are looking specifically to filter to cards that have at least one label that has a chosen name. Consider the following: Your code filters to cards that have a specific label (via filtering on the ID field). While not being explicitly what is described in #2, this is a useful too! Im happy with you to either change this PR to be in line with #2 or to change the display (and class name) to make it clearer that the filter provides the new functionality of filtering to a specific label. Let us know which you prefer. Secondly, please could I ask that you run your code on a test board. One of the other recent PRs has resulted in the This PR is showing a lot of promise. I look forward to seeing your next push |
'Select Label to filter by:', | ||
array_merge(array_keys($options), ['<- Back']) | ||
); | ||
$questionHelper = new QuestionHelper(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The display for the setup renders as follows:
Configure Filter: Has Label
Select Label to filter by:
[0] 1 Alpha
[1] 3 Alpha
[2] 4 Bravo
[3] <- Back
>
You can rely on the ChoiceQuestion
to do the option numbering for you
*/ | ||
static public function getName() | ||
{ | ||
return 'Has Label'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you choose to go implement the functionality in #2, then this should probably change to something like Label Name
to differentiate it from the alternate functionality of matching a specific Label.
See \AppBundle\Helper\CardFilter\LabelColourCardFilter
for reference
*/ | ||
public function getConfigDescription() | ||
{ | ||
return 'Label: '.$this->getTargetLabel()->getName(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you chose to go implement matching a specific label, then have a look at how the labels are rendered in \AppBundle\Command\CardFilterCommand::printFilteredCards()
This covers Labels without names and attempts to differentiate Labels by colour
No description provided.