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

Line Annotations #51

Closed
wants to merge 3 commits into from
Closed

Line Annotations #51

wants to merge 3 commits into from

Conversation

karlhorky
Copy link

@karlhorky karlhorky commented Feb 28, 2019

Hi! First of all, thanks for this great project! It's super useful.

I'm creating teaching materials (course material) with this tool, and I would like to annotate certain parts of the source code with arrows, indicators, labels and other diagram elements that are not in monospace font (see examples below) - some similarities to LaTeX brackets / parentheses / braces. I tried to use the current APIs, but I didn't get very far.

I ended up creating an MVP solution (described below) and I wanted to get feedback on it :)

TODO

Ideal Examples

screen shot 2019-02-26 at 15 03 20

what-is-an-xml-element

images

698932
css-selectors-lrg

Current MVP Solution

In order to do the very minimum of implementation work with the largest impact, I have created an MVP for only line annotations that allows for strings and React components to be passed via a new property called lineAnnotations in the step object, which is an object mapping lines to their annotation content.

The MVP is currently published under @upleveled/mdx-deck-code-surfer@0.6.3 (depends on @upleveled/code-surfer@0.6.1).

To try it out:

yarn add --dev @upleveled/mdx-deck-code-surfer

It allows for things like this:

kapture 2019-02-28 at 12 51 48

via this code:

<CodeSurfer
  title="Example of HTML code"
  code={require('!raw-loader!./first-example.html')}
  lang="markup"
  steps={[
    {},
    {
      lineAnnotations: {
        4: (
          <span style={{ fontFamily: 'BlinkMacSystemFont' }}>
            {' '}
            <Arrow
              angle={270}
              length={300}
              style={{
                verticalAlign: '-4px',
                width: '300px',
              }}
              color="white"
            /> Opening Tag
          </span>
        ),
        6: (
          <span style={{ fontFamily: 'BlinkMacSystemFont' }}>
            {' '}
            <Arrow
              angle={270}
              length={292}
              style={{
                verticalAlign: '-4px',
                width: '292px',
              }}
              color="white"
            /> Closing Tag
          </span>
        ),
      },
    },
  ]}
/>

What are your opinions of this API?

More specifically:

  1. Do you have any recommendations on a better way to achieve what I'm intending here?
  2. Should I use Scroller.Element at all?
  3. Should this API be more generalized? More like:
interface Step {
  annotations?: {
    lines?: {
      [key: number]: JSX.Element | string
    }
  }
}
  1. Should I update the step-parser.js test file?

@pomber
Copy link
Owner

pomber commented Mar 2, 2019

Yes! I'd love to have some kind of line annotations. They are much better than notes.
Could you add a slide to the sample deck in the PR so we can see it in action?

I don't have anwsers for your questions yet. I'm focusing on another project right now. I'll come back to code-surfer soon (in a few weeks) 'cause I need to write a talk and I want to rewrite a big part of this project to support new features.

@karlhorky
Copy link
Author

Great, glad that the feature is also valuable for others!

Could you add a slide to the sample deck in the PR so we can see it in action?

Sure, I've added a TODO in the description. I'll do this along with any feedback that you have in a few weeks :)

@karlhorky
Copy link
Author

I've also changed the lineAnnotations keys to be 1-based instead of 0-based, so that they match line numbers.

@karlhorky
Copy link
Author

I'll come back to code-surfer soon (in a few weeks)

Did you have a chance to put some more thought into this?

@pomber
Copy link
Owner

pomber commented May 10, 2019

Hi @karlhorky , I'm currently rewriting code surfer from scratch to support new things. Line annotations is one of those things I want to support. I'm keeping this PR open as a reference.
Thanks for your patience

@karlhorky
Copy link
Author

Ok, if you're reimplementing this feature, I may be able to be of assistance. Maybe it would make sense to make it more general and easier to work with. For example, general annotations for anything? Maybe with a default style for the "pointer"/"tooltip" element?

@pomber pomber mentioned this pull request May 13, 2019
19 tasks
@karlhorky
Copy link
Author

Hi @pomber ! Now that v3 has dropped, do you have any insights into how such a feature could be implemented there?

Would really like to do this to be able to annotate my code like in the examples above :)

@pomber
Copy link
Owner

pomber commented Dec 13, 2019

I want to use something like annotations for blogging too.

One approach I want to try is to provide a useStepInfo() hook so any child of the step can absolute-position itself in the right place of the slide. Then, export an Annotation component that use that hook together with the lines and columns that it's annotating to show the annotation.

But I don't have the time now, it would have to wait...

@karlhorky
Copy link
Author

Ok, so there's no way to do this at the moment, even with a more hacky solution?

I guess I'll have to wait to upgrade then... Or find some workaround...

@pomber
Copy link
Owner

pomber commented Dec 16, 2019

For now, you could try to write your own version of this component and hardcode the annotations there for each slide. But it may be too hacky...

@karlhorky
Copy link
Author

Ok, will take a look, maybe I can get something working. Thanks!

@karlhorky
Copy link
Author

Hey @pomber, I didn't get a chance to try this out yet... Any news from your side on this feature?

@pomber
Copy link
Owner

pomber commented Aug 12, 2020

Hey @pomber, I didn't get a chance to try this out yet... Any news from your side on this feature?

Hi. Not really. I'm focusing more on this other project, which eventually should replace the core of code surfer and be more flexible.

@karlhorky
Copy link
Author

Ok thanks. I guess probably no ETA for Code Hike?

code-surfer@0.6.13 is aging not so gracefully (still requires mdx-deck@^1).

Maybe time to try to upgrade one of my decks to 3.1.1 and try to come up with a new workaround.

@karlhorky
Copy link
Author

Ah just trying this out - unless I'm missing something I guess the new Code Surfer steps API is much less flexible :(

Maybe I move off of Code Surfer completely and try to implement my own thing using react-syntax-highlighter and useStep from mdx-deck.

@pomber
Copy link
Owner

pomber commented Aug 25, 2020

Maybe I move off of Code Surfer completely and try to implement my own thing using react-syntax-highlighter and useStep from mdx-deck.

That may be the best alternative if you need something soon

@karlhorky
Copy link
Author

Ok, I've opened up a followup to this in #115, maybe we can continue the discussion over there...

@karlhorky karlhorky closed this May 15, 2021
@karlhorky karlhorky deleted the line-annotations branch May 15, 2021 15:50
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