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

446 linkify username in comment #1686

Merged
merged 13 commits into from Sep 1, 2017
Merged

Conversation

relativityboy
Copy link
Contributor

@relativityboy relativityboy commented Aug 29, 2017

#446

Author name is now always a link. Default browser behavior on the links is canceled unless or keys are pressed while clicking. A toggle event for the popup is fired if those keys are not pressed.

@relativityboy relativityboy changed the title [WIP] 446 linkify username in comment 446 linkify username in comment Aug 29, 2017
@relativityboy relativityboy removed the WIP label Aug 29, 2017
@relativityboy relativityboy self-assigned this Aug 29, 2017
valzav
valzav previously approved these changes Aug 29, 2017
@relativityboy relativityboy changed the title 446 linkify username in comment [WIP] 446 linkify username in comment Aug 29, 2017
@relativityboy relativityboy changed the title [WIP] 446 linkify username in comment 446 linkify username in comment Aug 30, 2017
@relativityboy relativityboy removed the WIP label Aug 30, 2017
@sneak sneak dismissed valzav’s stale review August 31, 2017 11:32

there is a bug

@sneak
Copy link
Contributor

sneak commented Aug 31, 2017

There is a history bug.

Steps to reproduce:

  1. Open link to article page

  2. Click username to show popup

  3. click username in popup to go to blog page

  4. hit back

expected:

return to post page

actual:

nothing happens

(this is not in relation to the issue i filed with middle-clicking to make a new tab; just on the single, original tab.)

@relativityboy
Copy link
Contributor Author

@sneak there was a bug with addEventListener and removeEventListener not always being present. Added checks; that's fixed most of the "back" navigation problem.

Copy link
Contributor

@roadscape roadscape left a comment

Choose a reason for hiding this comment

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

Tested, working well. The only potential UI issue I see is that the author name is now underlined on hover -- I don't believe condenser uses any underlined links, hover or not.

@relativityboy
Copy link
Contributor Author

relativityboy commented Sep 1, 2017

@roadscape - there's variation in links. Some are underlined, some are not. I've added a bit of styling to remove the underline for these.

@roadscape roadscape merged commit 9a21604 into master Sep 1, 2017
@roadscape roadscape deleted the 446-linkify-username-in-root-comment branch September 1, 2017 17:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants