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

feature: render highlight color in article note #215

Merged
merged 1 commit into from
Apr 17, 2024

Conversation

aaachen
Copy link
Contributor

@aaachen aaachen commented Apr 10, 2024

Issue

Issue: #166

feature: render highlight color in article note
improvement: refactor settings tab into its own file

Description

Two options here for highlight css

  1. directly configure color mapping in Omnivore plugin, comes with rounded highlight but user can choose to customize in their css
  2. delegate color mapping to Highlightr plugin (by setting corresponding highlight css class). Benefit of this option is more highlight style the plugin supports, con is it's another dependency
  • Ensure to pick the css-class style option in Highlightr plugin

Caveat

  • Some themes (and plugins like Highlightr) may have global css setting that overwrite the css the omnivore plugin sets, be aware to customize around id. One such theme I've tested with is Prism

Testing

  • Unit Test ✅ - PASS src/tests/renderHighlightColorQuote.spec.ts (5.667 s)
  • Manual Test ✅ - See screenshots below

Screenshots

settings-1

  • Default setting does not render highlight color

settings-2

  • Settings (configure color mapping in omnivore plugin)

settings-3(highlightr)

  • Settings (configure color mapping in highlightr plugin)

article-1

  • Article page

article-2

  • Reference

highlightr-style

  • Using style of Highlightr plugin

settings-overview

  • Refactored settings page with section title grouping together the relevant information

Copy link
Collaborator

@sywhb sywhb left a comment

Choose a reason for hiding this comment

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

Look great to me. Really appreciate it as this is one of the biggest request from the users

@sywhb
Copy link
Collaborator

sywhb commented Apr 11, 2024

@aaachen Sorry I think the develop branch is a bit outdated due to some of the latest work pushed to master branch so I rebased the branch and it created some conflicts 😢

improvement: settings refactor
@aaachen aaachen reopened this Apr 16, 2024
@aaachen
Copy link
Contributor Author

aaachen commented Apr 16, 2024

@aaachen Sorry I think the develop branch is a bit outdated due to some of the latest work pushed to master branch so I rebased the branch and it created some conflicts 😢

@sywhb Fixed the conflicts in respect to master, lmk if you see any problem

p.s. also removed the "Settings for Omnivore Plugin" and "General" headers according to the guideline here

@sywhb
Copy link
Collaborator

sywhb commented Apr 17, 2024

@aaachen Sorry I think the develop branch is a bit outdated due to some of the latest work pushed to master branch so I rebased the branch and it created some conflicts 😢

@sywhb Fixed the conflicts in respect to master, lmk if you see any problem

p.s. also removed the "Settings for Omnivore Plugin" and "General" headers according to the guideline here

Looks great to me!

@sywhb sywhb merged commit 1cbfe80 into omnivore-app:develop Apr 17, 2024
12 checks passed
sywhb pushed a commit that referenced this pull request Apr 17, 2024
sywhb pushed a commit that referenced this pull request Apr 17, 2024
sywhb added a commit that referenced this pull request Apr 17, 2024
sywhb pushed a commit that referenced this pull request Apr 17, 2024
sywhb added a commit that referenced this pull request Apr 17, 2024
feat: render highlight color in article note (#215)
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.

2 participants