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

Allow posts listed in Mod Center to open in a new tab #8976

Closed
Turnerj opened this issue Jun 29, 2020 · 12 comments · Fixed by #9314
Closed

Allow posts listed in Mod Center to open in a new tab #8976

Turnerj opened this issue Jun 29, 2020 · 12 comments · Fixed by #9314
Assignees
Labels
area: moderation tooling, blocking, fighting spam or code of conduct abusers bug always open for contribution good first issue good first issues for anyone new to programming and new to the project.

Comments

@Turnerj
Copy link
Contributor

Turnerj commented Jun 29, 2020

Is your feature request related to a problem? Please describe.

Currently in the Mod center, when you click a post it opens inline underneath the post you clicked. This is really nice BUT it can be useful opening a bunch of posts into multiple tabs and handle it that way.

Because the post items are handled by JS magic, you can't actually open them into a new tab.

Describe the solution you'd like

So my thought is keep the inline JS stuff but have the post items be anchors so when we Ctrl+Click etc, it can open in a new tab. I'd probably go a bit further and make the link include a query string parameter or fragment which also indicates that the mod tools should open up by default on that link.

@jacobherrington jacobherrington added area: moderation tooling, blocking, fighting spam or code of conduct abusers tech: javascript bug always open for contribution labels Jun 29, 2020
@jacobherrington
Copy link
Contributor

Thanks for the issue! We'll take your request into consideration and follow up if we decide to tackle this issue.

To our amazing contributors: issues labeled type: bug are always up for grabs, but for feature requests, please wait until we add a ready for dev before starting to work on it.

To claim an issue to work on, please leave a comment. If you've claimed the issue and need help, please ping @thepracticaldev/oss and we will follow up within 3 business days.

For full info on how to contribute, please check out our contributors guide.

@jacobherrington jacobherrington added difficulty: easy good first issue good first issues for anyone new to programming and new to the project. labels Jun 29, 2020
@jacobherrington
Copy link
Contributor

I played around with this a little bit. I think the best solution is to add a link icon next to the title:
image

I did the above pretty quickly by changing the following lines:
https://github.com/thepracticaldev/dev.to/blob/56736aab1527c301b73fffce50b085c044145de4/app/javascript/modCenter/singleArticle/index.jsx#L53-L55

To something like this:

import ExternalLinkIcon from 'images/external-link.svg';
...
<h3 className="fs-base fw-bold lh-tight">
  {title}
  <a href={path}>
    <span className="crayons-icon">
      <img src={ExternalLinkIcon} aria-label="External Link" />
    </span>
  </a>
</h3>

@Turnerj
Copy link
Contributor Author

Turnerj commented Jun 30, 2020

Interesting - it does give the functionality though looking at the index.jsx markup, I'm not sure how well putting an anchor in a button would turn out (I didn't realise it was a button tag for the whole post before).

If it does work and doesn't seem too weird (anchor tag in button tag), I think it might be a little confusing as my first thought is that the whole title would be a link then. This would follow the similar design choice as shown on the "Listing" link in the dashboard:
image

@jacobherrington
Copy link
Contributor

@Turnerj Those are good points! We might want @ludwiczakpawel or @lisasy to weigh on the design here!

@Turnerj
Copy link
Contributor Author

Turnerj commented Jul 1, 2020

If it helps, looking at the list of posts in the Dashboard compared to the list of posts in the Mod Center, the Dashboard has the whole post title be a link. While the Dashboard doesn't have that inline preview that the Mod Center does, it seems to have set the design language for how to approach a list of posts.

@ludwiczakpawel
Copy link
Contributor

I think we should reconsider how the row in mod center is behaving. I'd personally not expect the whole row to be clickable but only the title itself (or specific controls).

Now if you click the title in the usual way, it should expand the preview panel below. But if you try to open it in a new tab it should definitely open the article page itself.

There's also an author name in the row and I think it should also be a clickable element but taking you to the user's profile...

I'd personally not make the whole row a button element because it's just off/weird and semantically incorrect I think.

@Turnerj
Copy link
Contributor Author

Turnerj commented Jul 1, 2020

I like those suggestions @ludwiczakpawel - I feel that those suggestions keep it consistent with other bits of UI across the site.

@ludwiczakpawel
Copy link
Contributor

@Turnerj yeap, we actually have similar "table" on Dashboard right now - and the whole row itself is not being clickable, only specific elements inside are clickable.

@PrimeNoodles
Copy link
Contributor

I can try to work on this issue, if I understand from what your suggesting, we should include an icon to link to a new tab right?

@nickytonline
Copy link
Contributor

Would you like me to assign this to you then @PrimeNoodles?

@PrimeNoodles
Copy link
Contributor

I'm still a little new with working with this code base, but I'll try, sure!

@nickytonline
Copy link
Contributor

Awesome @PrimeNoodles! 🔥 If you need any assistance, feel free to hit me up for help or anyone else on the team.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: moderation tooling, blocking, fighting spam or code of conduct abusers bug always open for contribution good first issue good first issues for anyone new to programming and new to the project.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants