Skip to content

Add a line to the code editor that indicates the 80th column#358

Open
NV wants to merge 1 commit into
montagestudio:masterfrom
NV:code-boundary-line
Open

Add a line to the code editor that indicates the 80th column#358
NV wants to merge 1 commit into
montagestudio:masterfrom
NV:code-boundary-line

Conversation

@NV

@NV NV commented May 12, 2014

Copy link
Copy Markdown
Contributor

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This doesn’t do anything, e.g. this <div> does not appear in the DOM. So I ended up appending it in JS; I’m sure there is a better way of doing this.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It needs to be inside the data-montage-id="code-editor" div, as that's the root element of the component

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Duh. Thanks, it works!

@Stuk

Stuk commented May 12, 2014

Copy link
Copy Markdown
Contributor

It breaks after doing Cmd++ once, although fixes after a second +. It would be nice if that worked, but not sure how hard that is...
test

@mczepiel

Copy link
Copy Markdown
Contributor

Ok, it seems completely misplaced to me at 100%. And even here it seems to clip a character slightly.

screen shot 2014-05-19 at 2 25 18 pm

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I suspect we'll need to actually measure the width of a character and set it explicitly.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Manually measuring with dev tools looks like 80 characters is 640px wide. 702px seems exactly like what I'd expect at 100%, but suffers the same 110% offset issue as noted by stuk.

Can we put the boundary line inside the code area so we don't need to account for the gutter in our positioning? Otherwise we nee to measure that to account for it's 50px offset.

Relevant: codemirror/codemirror5#541

I'd also be happy with a presentation of the column the cursor is in somewhere, we just don't have anywhere to display that easily right now.

And this thing isn't a real priority, I was hoping it would be quick.

EDIT: 50px gutter + 12px padding + 640px offset = 702px. Would be nice to not have to deal with all the offsets in placing this, might be less prone to breaking when scaling.
EDIT: Nope, no more prone to the scaling issue when it's "inside" well that's unfortunate.

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.

3 participants