feat: Add option for line-wrapping to prevent horizontal scrolling

[akoreman]

Description

Builds on top of #15.

When using code-view with lines longer than the width of the component, currently horizontal scroll bars are shown. This causes potential accessibility problems:

• code-view is not focusable, so keyboard users are unable to scroll the long lines into view.
• when code view is used in a vertically scrolling list, this might cause situations where users are required to scroll in two directions to scroll longs lines into view.

This adds a lineWrapping property to the API which, if set to true, makes the component line-wrap long lines to keep them within the view without scrolling.

Visual regression tests in the PR actions are failing because this PR makes visual changes.

Related links, issue #, if available: n/a

How has this been tested?

• adds unit tests testing that the right class is applied when lineWrapping is false, undefined or true
• adds page with different reflow scenarios with and without line numbers, allowing for visual and manual testing.

Review checklist

The following items are to be evaluated by the author(s) and the reviewer(s).

Correctness

• Changes include appropriate documentation updates.
• Changes are backward-compatible if not indicated, see CONTRIBUTING.md.
• Changes do not include unsupported browser features, see CONTRIBUTING.md.
• Changes were manually tested for accessibility, see accessibility guidelines.

Testing

• Changes are covered with new/existing unit tests?
• Changes are covered with new/existing integration tests?

---

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[just-boris]

Changing the markup here breaks selection feature. Line numbers get added into selection.

For the reference you can check how it works on Github

Screen.Recording.2024-02-25.at.16.11.35.mov

[akoreman]

ah, yeah that's a good catch, thanks. Added some CSS that should fix this.

[just-boris]

Why is this needed?

The original idea of this component to remove ace dependency from the bundle when syntax highlight is not used.

[akoreman]

I needed something to go from string to ReactElement for non-highlighted code, for this draft/POC I used the texthighlight rules from Ace for that because it's quick to grab. I can write a simple function within this repo to do the same thing before merging if we decide this approach for line-wrapping looks good.

[just-boris]

Thanks for the info. Let's ensure it gets removed at the final stage

[akoreman]

Removed it and replaced it with a function defined in the PR.

[rubencarvalho]

I'm looking at this PR.
From my first high-level look at it, there are a couple of blocking, open to-dos:

• Add a minimum height for when actions are present (so the actions have enough space when it is only 1 line)
• Actions absolute positioning is currently not relative to the code view block, so they scroll with the content
• Code identation was lost on the highlighting page

[rubencarvalho]

I'm looking at this PR. From my first high-level look at it, there are a couple of blocking, open to-dos:

• Add a minimum height for when actions are present (so the actions have enough space when it is only 1 line)
• Actions absolute positioning is currently not relative to the code view block, so they scroll with the content
• Code identation was lost on the highlighting page

I've went ahead and pushed a commit to tackle the initial two. I am looking into the last one now.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (2aa88d5) to head (5b7e6e6).

Additional details and impacted files

@@            Coverage Diff            @@
##              main       #20   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           27        27           
  Lines          348       381   +33     
  Branches        31        30    -1     
=========================================
+ Hits           348       381   +33     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[rubencarvalho]

I'm looking at this PR. From my first high-level look at it, there are a couple of blocking, open to-dos:

• Add a minimum height for when actions are present (so the actions have enough space when it is only 1 line)
• Actions absolute positioning is currently not relative to the code view block, so they scroll with the content
• Code identation was lost on the highlighting page

I've went ahead and pushed a commit to tackle the initial two. I am looking into the last one now.

I've fixed the code indentation by setting white-space to pre.

[gethinwebster]

is it necessary for this to be semantically a table? or is this more just to get table layout, in which case could we do that purely with CSS? (thinking about a11y, and whether it makes sense for this to be announced as a "table")

[rubencarvalho]

The primary reason for choosing a table here is to align line numbers with their corresponding lines of code, now that we are introducing line-wrapping. Let us consult with A11Y to double-check.