Edit TagGroup component to support onClick and displaying titles #129
Conversation
hstastna
force-pushed
the
Add_links_titles_for_TagGroup
branch
from
58ca058 to
417f655
Compare
hstastna
force-pushed
the
Add_links_titles_for_TagGroup
branch
from
417f655 to
df06f8d
Compare
hstastna
changed the title
hstastna
force-pushed
the
Add_links_titles_for_TagGroup
branch
3 times, most recently
from
c6578af to
2cc535a
Compare
|
@miq-bot add_reviewer @Hyperkid123 |
src/textual_summary/tag_group.jsx
Outdated
| <IconOrImage icon={subitem.icon} image={subitem.image} text={subitem.text} /> | ||
| {this.renderSubiteList(subitem)} | ||
| {Array.isArray(subitem.value) ? this.renderSubiteList(subitem) : this.renderSubitem(subitem)} |
There was a problem hiding this comment.
Can you fix this typo renderSubiteList? I know it was here before 😉
hstastna
force-pushed
the
Add_links_titles_for_TagGroup
branch
from
2cc535a to
1949ade
Compare
| */ | ||
| renderSubiteList = subitem => ( | ||
| renderSubitemList = subitem => ( |
src/textual_summary/tag_group.jsx
Outdated
| <IconOrImage icon={item.icon} image={item.image} title={item.title} /> | ||
| {item.value} | ||
| <td title={item.title} onClick={e => this.checkLinkItem(item, e)}> | ||
| <IconOrImage icon={item.icon} image={item.image} title={item.title} /> {item.value} |
There was a problem hiding this comment.
keep the 2 lines as 2 lines please :)
(otherwise, {item.value} is completely hidden in the code)
There was a problem hiding this comment.
Do you think it is better to add before {item.value} if I want to achieve a nice space between icon and the item name? Like this:
Because I think we don't want this:
Moving {item.value} to the same line as IconOrImage was the simplest and working fix for this.
There was a problem hiding this comment.
Ah, sorry, react jsx is not html...
I think the canonical way to do this (if I understand facebook/jsx#19 (comment) right) is..
<td title={item.title} onClick={e => this.checkLinkItem(item, e)}>
<IconOrImage icon={item.icon} image={item.image} title={item.title} />
{' '}
{item.value}
</td>up to you :)
|
Overall, matches the other components which already do the same thing 👍 (I don't agree that onClick is ever the right way to go to a different screen, we should be using proper links, but that's a separate issue.) |
hstastna
force-pushed
the
Add_links_titles_for_TagGroup
branch
from
1949ade to
64486c6
Compare
|
@himdel : all feedback properly adressed? |
hstastna
force-pushed
the
Add_links_titles_for_TagGroup
branch
from
64486c6 to
740737b
Compare
|
@martinpovolny No real issues from me, only #129 (comment) but it's not really that important. |
hstastna
force-pushed
the
Add_links_titles_for_TagGroup
branch
from
740737b to
9a34979
Compare
|
Checked commits hstastna/react-ui-components@0837a10~...9a34979 with ruby 2.4.6, rubocop 0.69.0, haml-lint 0.20.0, and yamllint 1.10.0 |
|
🎉 This PR is included in version 0.11.28 🎉 The release is available on: Your semantic-release bot 📦🚀 |
What:
Edit
TagGroupcomponent to supportonClickand displaying titles (if they are specified).Add some data for the appropriate storybook.
Why:
Needed for ManageIQ/manageiq-ui-classic#5675
Related RFE:
https://bugzilla.redhat.com/show_bug.cgi?id=1678124
Some screenshots at:
ManageIQ/manageiq-ui-classic#5675