-
Notifications
You must be signed in to change notification settings - Fork 10
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/replace-get-highlighted-text #313
Open
RobHelgeson
wants to merge
42
commits into
mortii:v4
Choose a base branch
from
RobHelgeson:feature/replace-get-highlighted-text
base: v4
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from 1 commit
Commits
Show all changes
42 commits
Select commit
Hold shift + click to select a range
663b04e
Add JIT highlighting of morphs
RobHelgeson 05ff0b6
Add Rubification of sting - built in filters cannot run after custom …
RobHelgeson 2df17f9
Allow filter to run after built-in filter
RobHelgeson 9056db7
Add documentation
RobHelgeson d84799b
Fix am-highlight-morphs parse html issue
RobHelgeson 1117111
Allow cards where you want to use am-highlight-morphs to not have the…
RobHelgeson 3401040
Address PR comments
RobHelgeson bd73b95
Update documentation
RobHelgeson d2e2cf4
Update documentation
RobHelgeson 0e2573a
Updates for code review
RobHelgeson fdeb76e
Fix bad morph highlighting in ruby html
RobHelgeson 832a255
Fix bad morph highlighting in ruby html - WIP
RobHelgeson b23ebd8
experemental
RobHelgeson 005f2dd
Reimpl of highlight text using regexes.
RobHelgeson eb34b3a
Update highlight comments.
RobHelgeson ea14846
Update documentation.
RobHelgeson 6072576
WIP - WIP
RobHelgeson fb5ce61
Update jit formatter with changes from experimental branch
RobHelgeson 7189712
Checkpoint commit - WIP
RobHelgeson 7a3ae5f
Checkpoint speeeeedy! - WIP
RobHelgeson f7841ee
Checkpoint - WIP
RobHelgeson c8e8883
Checkpoint - WIP
RobHelgeson 994355f
Checkpoint - WIP
RobHelgeson fdc6a07
Checkpoint - WIP
RobHelgeson c16ba70
Checkpoint - feature complete
RobHelgeson 78aee2e
Add JIT highlighting
RobHelgeson eb57cba
Documentation and dead code
RobHelgeson f2b702f
Cleanup
RobHelgeson 47f7c14
Replace get_highlighted_text
RobHelgeson b5af09f
Rename some variables for clarity
RobHelgeson 20336c1
Formatting per PR request.
RobHelgeson da5eb66
Renamed some symbols for clarity.
RobHelgeson 4b4be12
Renamed some symbols for clarity.
RobHelgeson b530660
updated guide
mortii f8c56d1
fixed broken test
mortii d569b52
deleted empty file
mortii e58e9d7
Address text parsing bug where rubies were allowed to overlap
RobHelgeson af3449b
Support text based rubies
RobHelgeson 0d897a4
Pr cleanup
RobHelgeson bda97d0
Implement jit formatting for furigana, kanji and kana.
RobHelgeson c4eb14d
added temp verbose dev logging
mortii 648054c
Create subclasses for all text formatting options
RobHelgeson File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think you can inline styles like this... It doesn't work for me at least.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Odd, this worked for me.
I must have some other things allowing it to work. I'll double check.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you send me either a screenshot, or the html, produced by (something like):
I'm trying to break it over here but it looks like it works for me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does the font size change if you do this?
do you have a css-injector add-on activated?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
font-size: 999;
the 999 needs a measurement likerem
,px
or%
.Edit: yes changing font size does affect the output.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right, does a valid change like that work for you?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I get the same result if I remove the styles:
If i change the font size nothing happens.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😮
that's... odd.
is it possible that you may have some of the css that we were experimenting with before, still active somewhere?
I'll attach some screenshots, if you can try this, we can see where your styles are coming from vs where mine are coming from.
grab: AnkiWebView Inspector https://ankiweb.net/shared/info/31746032
inspect kana element by right clicking and inspecting:
click Computed in the right panel of the inspector:
click the
circled arrow
icon on the property you want to see where it "came from" (I cannot screenshot the circled icon because it's on mouseover only, but its a grey circle next to the value of the property):this will take you to the
styles
tab, where you can see what styles are loaded and applied:and by clicking on the top right
<style>
here, it will highlight where that style got loaded in.for me, that is this block:
I know that's a HUGE pain in the butt. :D but I don't know why this would not be working for you but is for me (so far)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aha, I realize why.
When using
{{am-highlight-kana:kana:Front}}
I don't get any ruby/rt elements at all:but I do get them when only using
{{am-highlight-kana:Front}}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
aha, this makes sense. The built-in filter removes all kanji from the string before we even "see it" so we process a "kana only" string.
This has the negative side effect of not allowing us to use
{{am-highlight-kana:kana:Front}}
for backward compatibility (i.e. when on mobile).The built in
furigana
andkanji
filters will still work because the morphs match on kanji (most of the time) with kana readings removed from the parsed string.I can't speak for you the project, but I'll suggest that having that as a known issue may be the appropriate action, with the recommendation to either:
{{am-highlight-kana:furigana:Front}}
and have kana locally and furigana on mobile.{{am-highlight-kana:Front}}
locally and{{kana:Front}}
on mobile (I do not know how to do this at the moment)