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

Make dim-visited-conversations a CSS feature #5305

Merged
merged 5 commits into from Jan 18, 2022
Merged

Conversation

kidonng
Copy link
Member

@kidonng kidonng commented Jan 14, 2022

Proposed to fix #5299

Make #5258 a standalone CSS feature, and disable it on repos the user has write access to.

Test URLs

https://github.com/refined-github/refined-github/issues or any repo you can edit

Screenshot

N/A

@cheap-glitch
Copy link
Member

I still feel like this is too opinionated to be a RGH feature, even if it's restricted to repos you don't own ­— which is the majority of them.

But let's see what people think 😃

@kidonng

This comment has been minimized.

@fregante
Copy link
Member

fregante commented Jan 15, 2022

Sorry this doesn't make sense. :visited is a native browser feature. I don't see the need to customize it any further: not on the user's status, not on the link's hash part.

@fregante fregante closed this Jan 15, 2022
@fregante fregante deleted the light-up-if-you-can-edit branch January 15, 2022 12:35
@kidonng
Copy link
Member Author

kidonng commented Jan 15, 2022

This is only a proposed solution, not the final decision. The need to make it a standalone option (or drop it) is still there.

@kidonng kidonng restored the light-up-if-you-can-edit branch January 15, 2022 18:36
@kidonng kidonng changed the title Don't dim visited conversations if you can edit the repo Make dim-visited-conversations a CSS feature Jan 15, 2022
@kidonng kidonng reopened this Jan 15, 2022
@kidonng kidonng marked this pull request as ready for review January 15, 2022 18:55
@@ -267,7 +267,7 @@ Thanks for contributing! 🦋🙌
- [](# "dim-bots") [Dims commits and PRs by bots to reduce noise.](https://user-images.githubusercontent.com/1402241/65263190-44c52b00-db36-11e9-9b33-d275d3c8479d.gif)
- [](# "esc-to-cancel") [Adds a shortcut to cancel editing a conversation title: <kbd>esc</kbd>.](https://user-images.githubusercontent.com/35100156/98303086-d81d2200-1fbd-11eb-8529-70d48d889bcf.gif)
- [](# "no-duplicate-list-update-time") [Hides the update time of conversations in lists when it matches the open/closed/merged time.](https://user-images.githubusercontent.com/1402241/111357166-ac3a3900-864e-11eb-884a-d6d6da88f7e2.png)
- [](# "dim-visited-conversations") [Dims the title of visited conversations, unless you have write access to the repo.](https://user-images.githubusercontent.com/44045911/148573545-39a72dab-a37a-491d-83f3-d6fd015f5dcf.mov)
- [](# "dim-visited-conversations") [Dims the title of visited conversations.](https://user-images.githubusercontent.com/44045911/149634307-1ed0e554-6957-4afa-8458-9c5bb4f80ebb.mp4)
Copy link
Member

Choose a reason for hiding this comment

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

The video does not load in Safari for whatever reason. I think it's best to stick with gifs and revert the previous update. I don't want to have to deal with codec issues, gifs just work

Screen Shot

Copy link
Member Author

@kidonng kidonng Jan 19, 2022

Choose a reason for hiding this comment

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

Previous video featured hash changes which is no longer present, so I rerecorded the video.

And I took the chance to convert it into .mp4 with ffmpeg since Chrome doesn't open .mov videos if opened directly (or in our case, click the link in README). .mov videos embedded on GitHub are fine though.

I'm surprised that Safari doesn't support the converted video because macOS does. Maybe there's something wrong with how I converted it, I will take a look later today.

I think it's best to stick with gifs and revert the previous update. I don't want to have to deal with codec issues, gifs just work

I don't want to deal with codec issues but perhaps it's my fault here, not the browser's. Ditching entire support for videos just because of this seems too much though.

Copy link
Member

Choose a reason for hiding this comment

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

It's not too much because, again, I don't want to have debug/review this in the future. Gifs made with licecap are super lightweight too

@fregante fregante merged commit 7a73b13 into main Jan 18, 2022
@fregante fregante deleted the light-up-if-you-can-edit branch January 18, 2022 15:35
@fregante
Copy link
Member

Merging this but the video should still be undone in the lint PR or a PR to revert the whole video feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Issue titles in issues tab are dimmed / gray
3 participants