-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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 Filter Object to Multi Conversation Select Block Element #1191
base: master
Are you sure you want to change the base?
Add Filter Object to Multi Conversation Select Block Element #1191
Conversation
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.
LGTM! Thanks :)
block_element.go
Outdated
@@ -279,6 +279,7 @@ type MultiSelectBlockElement struct { | |||
InitialChannels []string `json:"initial_channels,omitempty"` | |||
MinQueryLength *int `json:"min_query_length,omitempty"` | |||
MaxSelectedItems *int `json:"max_selected_items,omitempty"` | |||
Filter *SelectBlockElementFilter `json:"filter,omitempty"` |
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.
[nits]
We should rename to ConversationListsFilter
.
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.
Hi @kanata2 i've updated to be ConversationListsFilter
, kindly help to check again :)
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.
(Sorry, this suggestion is backward imcompatible change... I'll make new minor version and include this.)
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.
Btw is there anything that i need to do ? or just waiting for this MR merged by you? @kanata2
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.
I'll merge and release as soon as it's ready.
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.
Sorry, this was a mis-communication; it should be Filter
-> ConversationListFilter
. We definitely don't want to rename the existing SelectBlockElementFilter
. If you want to correct this and the linting issues I'll have another look.
PR preparation
make pr-prep
from the root of the repository to run formatting, linting and tests.Should this be an issue instead
API changes
N/A
This changes is to add the
filter
object inmulti_conversation_select
block element type since the documentation said thefilter
object is also supported in that type. Currently thefilter
object only present inconversation_select
type.Reference to the documentation: