Skip to content

chore(messages): migrate demos from radio to select #525

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Apr 18, 2025
Merged

chore(messages): migrate demos from radio to select #525

merged 5 commits into from
Apr 18, 2025

Conversation

Mash707
Copy link
Contributor

@Mash707 Mash707 commented Apr 17, 2025 •
edited
Loading

Closes: #433
The following examples will be migrated:

  • Bot Messages
  • User Messages
  • Attachment label

@patternfly-build
Copy link

patternfly-build commented Apr 17, 2025 •
edited
Loading

Preview: https://chatbot-pr-chatbot-525.surge.sh

A11y report: https://chatbot-pr-chatbot-525-a11y.surge.sh

@Mash707
Copy link
Contributor Author

Mash707 commented Apr 17, 2025 •
edited
Loading

@rebeccaalpert I have made changes to Bot messages only for now. Once we finalize it, I'll make similar changes for other examples too. Also need to work on the UI, it broke a little after I migrated to select
Let me if I am missing any other examples.

@rebeccaalpert
Copy link
Member

Just a couple of small suggestions! Otherwise looks good to me. Thanks for the contribution!

@rebeccaalpert rebeccaalpert requested a review from edonehoo April 17, 2025 18:42
@Mash707
Copy link
Contributor Author

Mash707 commented Apr 17, 2025

image
image
One small thing I have moved the "Message content type" heading inside the menu toggle. Let me know what you think, do I need to specify it as a separate heading outside the menu toggle?

@Mash707
Copy link
Contributor Author

Mash707 commented Apr 17, 2025

I have migrated all the related demos.

@Mash707 Mash707 requested a review from rebeccaalpert April 17, 2025 22:31
rebeccaalpert
rebeccaalpert requested changes Apr 18, 2025
View reviewed changes
Copy link
Member

@rebeccaalpert rebeccaalpert left a comment

Choose a reason for hiding this comment

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

Would you be willing to change the dropdown item values so they match the visible text in the dropdown? Otherwise the selected value is different than the dropdown value. I'm sorry if I missed this before. Everything else looks spot-on. Thank you.

Screenshot 2025-04-18 at 11 00 34 AM

@Mash707
Copy link
Contributor Author

Mash707 commented Apr 18, 2025 •
edited
Loading

Would you be willing to change the dropdown item values so they match the visible text in the dropdown?

Nice catch, I also missed it. Made the neccessary changes.

@Mash707 Mash707 force-pushed the migrate-message-demo-from-radio-to-select branch from e36a95e to f15d678 Compare April 18, 2025 18:27
@Mash707 Mash707 requested a review from rebeccaalpert April 18, 2025 18:29
rebeccaalpert
rebeccaalpert requested changes Apr 18, 2025
View reviewed changes
Copy link
Member

@rebeccaalpert rebeccaalpert left a comment

Choose a reason for hiding this comment

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

Just some small case differences that I know we'll get dinged for when our docs folks look this over. Otherwise everything looks great. Once these are in, I'm more than happy to merge this. Thank you so much for all your work so far.

@Mash707 Mash707 requested a review from rebeccaalpert April 18, 2025 20:06
@Mash707
Copy link
Contributor Author

Mash707 commented Apr 18, 2025

My camel casing habbit overlooked these 😅.

rebeccaalpert
rebeccaalpert approved these changes Apr 18, 2025
View reviewed changes
Copy link
Member

@rebeccaalpert rebeccaalpert left a comment

Choose a reason for hiding this comment

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

Thank you so much! 🚀

@rebeccaalpert rebeccaalpert merged commit 5ac90f9 into patternfly:main Apr 18, 2025
5 checks passed
@github-actions GitHub Actions
Copy link

🎉 This PR is included in version 6.3.0-prerelease.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@Mash707 Mash707 deleted the migrate-message-demo-from-radio-to-select branch April 23, 2025 19:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rebeccaalpert rebeccaalpert rebeccaalpert approved these changes

@edonehoo edonehoo Awaiting requested review from edonehoo

Assignees
No one assigned
Labels
released on @prerelease
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Migrate Message demos from radio to select
3 participants
@Mash707 @patternfly-build @rebeccaalpert