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: add config option to reorder items in emoji picker #215

Merged
merged 1 commit into from
Sep 28, 2024

Conversation

coder-with-a-bushido
Copy link
Contributor

No description provided.

Comment on lines 112 to 116
itemsOrderConfig: const ItemsOrderConfig(
top: EmojiPickerItem.categoryBar,
middle: EmojiPickerItem.emojiView,
bottom: EmojiPickerItem.searchBar,
),
Copy link
Owner

Choose a reason for hiding this comment

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

Nit: Might would use the term view instead of item, what do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Comment on lines 194 to 198
itemsOrderConfig: const ItemsOrderConfig(
top: EmojiPickerItem.categoryBar,
middle: EmojiPickerItem.emojiView,
bottom: EmojiPickerItem.searchBar,
),
emojiTextStyle: _textStyle,
emojiViewConfig: const EmojiViewConfig(
backgroundColor: Colors.white,
),
swapCategoryAndBottomBar: true,
Copy link
Owner

Choose a reason for hiding this comment

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

Updating the ReadMe would be appreciated

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@Fintasys
Copy link
Owner

Thank you for your PR - the code looks mostly good. I couldn't imagine that anyone would have the emoji view else where than in the middle, so I would be interested what has motivated you to make this change?

@coder-with-a-bushido
Copy link
Contributor Author

Thank you for your PR - the code looks mostly good. I couldn't imagine that anyone would have the emoji view else where than in the middle, so I would be interested what has motivated you to make this change?

Hey @Fintasys, thanks for the review!
I have addressed the comments now (sorry I was sick for a week, so I couldn't do it any soon).

As for the motivation behind this PR, it's because the design for an app I'm working on demanded this order - category view, search bar, emoji view.

Copy link
Owner

@Fintasys Fintasys left a comment

Choose a reason for hiding this comment

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

LGTM

@Fintasys Fintasys merged commit 7d21c79 into Fintasys:master Sep 28, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants