fedora-hubs

#589 make whole feed item a link
Merged by abompard. Opened by ryanlerch.
ryanlerch/fedora-hubs feed-links  into  develop

Download 589.patch

this makes the feeds use the bootstrap list-groups,
and makes the whole feed item a link to the thing that
relates to the feed item. Other links will still work inside
the item, like username urls and buttons.

fixes #562

Should we do something better when we have no link property? This will scroll the page back to the top and add a hash... I see two options:
- use a div and not a a when there's no link property
- use a onClick handler like: onClick={(e) => {e.preventDefault()}}.

The second one may be easier.

This seems to work:

--- a/js/app/components/feed/Panel.js
+++ b/js/app/components/feed/Panel.js
@@ -12,6 +12,7 @@ export default class Panel extends React.Component {
       detailsOpened: false,
     }
     this.toggleDetails = this.toggleDetails.bind(this);
+    this.handleClick = this.handleClick.bind(this);
   }
   toggleDetails(e) {
@@ -21,6 +22,12 @@ export default class Panel extends React.Component {
     ));
   }
+  handleClick(e) {
+    if (!this.props.item.link) {
+      e.preventDefault();
+    }
+  }
+
   render() {
     return (
       <a 
@@ -28,6 +35,7 @@ export default class Panel extends React.Component {
         id={this.props.item.id}
         href={this.props.item.link ? this.props.item.link : '#'}
         target="_blank"
+        onClick={this.handleClick}
         >
         <Icon item={this.props.item} />
         <Markup
@@ -42,7 +50,7 @@ export default class Panel extends React.Component {
              Object.keys(this.props.item.msg_ids).length > 1) &&
             <div className="mt-auto">
               <small>
-                <button className="btn btn-link" onClick={this.toggleDetails}>details</button>
+                <button className="btn btn-link p-0" onClick={this.toggleDetails}>details</button>
               </small>
             </div>
           }

I also aligned the "details" link with the "save" button, it looked weird.

And this seems to work too. It's a bit prettier because it does not generate a tags, so the cursor does not turn into a pointer on hover.

--- a/js/app/components/feed/Panel.js
+++ b/js/app/components/feed/Panel.js
@@ -22,12 +22,23 @@ export default class Panel extends React.Component {
   }
   render() {
+    let Element, props;
+    if (this.props.item.link) {
+      Element = "a";
+      props = {
+        className: "list-group-item list-group-item-action d-flex",
+        href: this.props.item.link,
+        target: "_blank",
+      };
+    } else {
+      Element = "div";
+      props = {
+        className: "list-group-item d-flex",
+      };
+    }
     return (
-      <a 
-        className="list-group-item list-group-item-action d-flex" 
+      <Element {...props}
         id={this.props.item.id}
-        href={this.props.item.link ? this.props.item.link : '#'}
-        target="_blank"
         >
         <Icon item={this.props.item} />
         <Markup
@@ -42,12 +53,12 @@ export default class Panel extends React.Component {
              Object.keys(this.props.item.msg_ids).length > 1) &&
             <div className="mt-auto">
               <small>
-                <button className="btn btn-link" onClick={this.toggleDetails}>details</button>
+                <button className="btn btn-link p-0" onClick={this.toggleDetails}>details</button>
               </small>
             </div>
           }
         </div>
-      </a>
+      </Element>
     );
   }

Your choice! :-)

Thanks @abompard

yeah, i think the second option is the wayto go! i tried to do this, but couldnt figure it out! didnt know about that Element trick!

thanks! applied the second patch, and rebased.

cheers,
ryanlerch

rebased onto 0a7dfc69b207edb06265a786b9727f265c6e3b4f

rebased onto fd4c7109534e1180994697a146f6058c570214d8

Pull-Request has been merged by abompard
Metadata
None
No Tags
Static archive. pagure.io was sunset in 2026; this is a read-only snapshot.
Red Hat Logo

Fedora is sponsored by Red Hat. Learn more about the relationship between Red Hat and Fedora »

© Red Hat, Inc. and others.