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

10035 text anchors with morphs do not render in rubric #10225

Conversation

kasperosterbye
Copy link
Contributor

After a lot of poking around, the problem was that RubAbstractTextArea simply did not render TextAnchors if they contained a morph. This PR fixes it (within my understanding of Rubrics).

@guillep
Copy link
Member

guillep commented Nov 16, 2021

Hi @kasperosterbye , could you manage to write a test on this somehow?

@kasperosterbye
Copy link
Contributor Author

I'll try - it is not obvious how it should be done as the layout algorithm actually made room for the morph, it just did not include it in the final rendering.
I'll look it over once more.

@Ducasse
Copy link
Member

Ducasse commented Nov 17, 2021

Thanks this is cool to have tests!!!

@kasperosterbye
Copy link
Contributor Author

I did manage to dream up something I think actually tests it.
The CI build fails for OSX, but ok for windows and linux. I do not understand it well enough to say if it is significant.
@guillep - can you at some point look at the test I wrote and tell me if it is actually test

@guillep
Copy link
Member

guillep commented Nov 17, 2021

Thanks @kasperosterbye for the quick response, I'll take a look in the next two days :)

@kasperosterbye
Copy link
Contributor Author

If you get the chance to look it over @guillep I would really appreciate it. The change is fundamental to the next version of in-image rendering of Microdown.

@guillep
Copy link
Member

guillep commented Nov 26, 2021

Yes, i'll check it after lunch during the sprint!

@MarcusDenker MarcusDenker linked an issue Nov 26, 2021 that may be closed by this pull request
@kasperosterbye
Copy link
Contributor Author

I like the the tearDown solution - thanks.

@@ -59,6 +59,31 @@ RubEditingAreaTest >> setUp [
area setTextWith: 'one two three four'.
]

{ #category : #'tests - accessing selection' }
Copy link
Member

@astares astares Nov 26, 2021

Choose a reason for hiding this comment

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

No dots after last expression at end of method necessary. There is nothing that should follow a tearDown in a tearDown method

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I know - they are not warned against, and when I edit a method they sometimes end up being left behind. Just like the extra spaces at the end of a line which are (were) inserted by the completion mechanism. It would be really useful to have an editor option to remove unnecessary dots and blanks at accept.

Copy link
Member

Choose a reason for hiding this comment

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

Yes - it is already on my "cleanup keep/improve code quality level" TODO list

RubEditingAreaTest >> tearDown [

area delete.
super tearDown.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

And an other dot at the end of the last statement. It was in your commit.

@kasperosterbye
Copy link
Contributor Author

I am afraid my git and iceberg skills are lacking here. I do not know how to include your commit in my PR so I can remove the extra dots.

@guillep
Copy link
Member

guillep commented Nov 26, 2021

I'm doing it quickly, I already have the environment ready :)

@Ducasse Ducasse merged commit 49c898b into pharo-project:Pharo10 Nov 26, 2021
@kasperosterbye kasperosterbye deleted the 10035-TextAnchors-with-morphs-do-not-render-in-Rubric branch June 21, 2022 05:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TextAnchors with morphs do not render in Rubric
5 participants