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

Some doubts about my recent PRs #113

Open
paean99 opened this issue Dec 29, 2018 · 1 comment
Open

Some doubts about my recent PRs #113

paean99 opened this issue Dec 29, 2018 · 1 comment

Comments

@paean99
Copy link
Contributor

paean99 commented Dec 29, 2018

The empty style sheet that was added to solve the nextjs bug seem to not work very well. I experienced several time, that when i click in the first link and right after a cold start of codeponder, that it can take several second for the page to change. It happens only the first time and not every restart. I have not found a pattern yet, but i believe that it may be related to the state of the nextjs build.

Another thing that seem to not be working in the best way: Some times after the codeponder starts and also only the first code page to be accessed, the syntax highlighting is not working. Just refreshing the page solves it, but i am not sure why it is happening. Probably also because of nextjs ssr.

A feature is missing from the line number input, they cannot reset themselves. At this moment the only way is through the mouse clicks. The easiest and simpler solution is adding a reset button to the form, but i am not sure what you envision for this feature. One alternative would be to put one of the input to '0'. It shouldn't be problematic in any way to implement.
One feature i added and probably not mentioned is that it is impossible to input numbers lesser (negative) or bigger than the number of lines in the code (need more work, since the code line lenght is calculated another time, even though react renderer does it already).

As for the line number mouse selection, i tried to get as near as i could to the github mouse selection functionality. But although i tried to let the code be as simple and straightforward as possible, i never explained the behavior. So i will take this space to elaborate a little.

  • each number clicked will have a toggle on/off behavior (reset). So clicking a second time in one of the numbers bordering the selected lines will disable it and leave only the other number selected if he exist.
  • the selection of the lines will be always between the last number clicked and the smallest number selected

I have been thinking that a "code ui component" (for storybook) should be the next iteration. It could be reused even in the comment and styled in a more controlled way.
But after seeing #109 (exited to see it released or at least some variant of it), i think that it should wait for the PR. Because it would imply to move the react renderer package to packages/ui to avoid the same problem as styled components and it would surely create to much code conflict with @onemem work.

@benawad
Copy link
Owner

benawad commented Dec 29, 2018

I've noticed weird stuff happening too and wasn't sure if it's a bug with the css import, ssr, or how we load prism. I wonder if this will show up in the production build.

I'm not exactly sure how I want to handle the line numbers yet, I think I will leave that last.

I think waiting is a good idea and I think it's ok to put the shareable component in the web package instead of the ui one

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

No branches or pull requests

2 participants