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

Provide community favorites shortcut #385

Closed

Conversation

naomiceron
Copy link

Motivation and Context

This adds a generalized concept of community favorites, which includes the existing focus shortcut and the new lofi girl shortcut.
Closes #374 (Add shortcuts for "community favorites")

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Description

This change adds a shortcut for community favorites as a general concept, clicking it makes appear a shortcut for focus and a shortcut for lofi. Tested it on Safari and Chrome.
Screen Shot 2021-12-26 at 11 56 36

Final checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING guidelines.
  • All tests passed.

@shakeelmohamed shakeelmohamed self-assigned this Jan 8, 2022
@shakeelmohamed
Copy link
Member

Thanks for the PR @naomiceron, pardon my delay in reviewing.
Functionality seems solid, mainly two pieces of feedback.

  1. I think this extra nested layer of the community favorite button can be removed though.

Screen Shot 2022-01-19 at 1 46 13 PM

  1. I noticed there’s a gap here that doesn’t look quite right

Screen Shot 2022-01-19 at 1 45 55 PM

display: block;
margin: 0 auto;
width: fit-content;
}

#focus-btn, #lofi-btn {
Copy link
Member

Choose a reason for hiding this comment

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

this should be a CSS class instead of two IDs

@@ -667,6 +669,13 @@ $(function() {
}
});

// Handle community favorites click
$("#fav-btn").click(function(event) {
Copy link
Member

Choose a reason for hiding this comment

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

Can refactor away this button

@naomiceron
Copy link
Author

Thank you! @shakeelmohamed, should I remove the community button entirely? I ask because of the second point

I noticed there’s a gap here that doesn’t look quite right.

I will make the changes as soon as possible :)

@shakeelmohamed
Copy link
Member

Thank you! @shakeelmohamed, should I remove the community button entirely? I ask because of the second point

Yeah that makes sense to me

@jbecker7
Copy link

jbecker7 commented Oct 3, 2022

@shakeelmohamed Do you still have any interest in having this implemented? I would be happy to work on it if you can think of buttons you would like :)

@shakeelmohamed
Copy link
Member

@jbecker7 Hey welcome! Go for it, please read through all comments in this PR and look at the changes before submitting your own

@shakeelmohamed
Copy link
Member

Closing due to no activity

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

Successfully merging this pull request may close these issues.

Add shortcuts for "community favorites"
3 participants