feat(ConversationHistoryNav): Allow support for favorites, tooltips, etc.#653
Conversation
|
Preview: https://chatbot-pr-chatbot-653.surge.sh A11y report: https://chatbot-pr-chatbot-653-a11y.surge.sh |
rebeccaalpert
force-pushed
the
custom-nodes
branch
4 times, most recently
from
ccba122 to
bb5b171
Compare
Revert back to menu implementation and ensure props passage down to appropriate Menu components to enable behavior.
rebeccaalpert
force-pushed
the
custom-nodes
branch
from
bb5b171 to
398a4af
Compare
rebeccaalpert
changed the title
There was a problem hiding this comment.
File comment below. Overall I'm good with reverting back to the Menu implementation - while IFD findings mentioned a concern about this section of the Chatbot being a menu, I do think it makes sense semantically (it's a list of options/actions you select from). The main reason for moving away from Menu was due to the inline "rename" input ask; since that pivoted to a Modal approach, using Menu should be fine (barring any future asks that may cause issues with using menu roles, but that's a problem for future us).
One thing I think I'd be a little confused about is the "enabling favorites" aspect of the linked issue. Is there a difference that we are (or would) convey between "favoriting" and "pinning" something? Typically the PF component favorite implementations are setup in a way where favoriting something in an example will pin it to the top of the list (exception being Table, but that's within a sortable implementation as well so you can manually sort by favorited or not favorited).
We may just need to tweak some verbiage in the "pin conversations" example since no net new logic was added at a component level - replace instances of "pin" with "favorite" - but just wanted to mention it since we did recently add an example specifically for what is essentially favoriting something (difference being that in the example Melissa bult out, the pin action is inside a kebab vs a dedicated "favorite" button typically seen in Menu examples).
| <MenuGroup | ||
| className="pf-chatbot__menu-item-header" | ||
| label={navGroup} | ||
| key={navGroup} | ||
| {...menuGroupProps?.[navGroup]} | ||
| > |
There was a problem hiding this comment.
I think we should default the heading level of the MenuGroups to an H3. From what I can tell it seems like in our examples this is rendered after an H2 "Chat history" heading, so these groups should be sub-sections of that.
There was a problem hiding this comment.
Good call-out! All set.
|
🎉 This PR is included in version 6.4.0-prerelease.15 🎉 The release is available on: Your semantic-release bot 📦🚀 |
…etc. (patternfly#653) Revert back to menu implementation and ensure props passage down to appropriate Menu components to enable behavior.
3 participants
With pass-through props and swap back to Menu implementation:
