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

Allow larger code blocks #777

Closed
cellio opened this issue Mar 1, 2022 · 6 comments
Closed

Allow larger code blocks #777

cellio opened this issue Mar 1, 2022 · 6 comments
Assignees
Labels
area: html/css/js Changes to front-end code complexity: easy Issues that should take limited effort to resolve/fix/build. priority: high type: change request New feature or request

Comments

@cellio
Copy link
Member

cellio commented Mar 1, 2022

https://software.codidact.com/posts/278868 was asked a year ago and it looks like we missed it then (I couldn't find an issue for it).

The code block is sized to show about 13 lines of code before vertical scrolling is needed. This impedes reading, particularly on the "code reviews" category on Software Dev where longer code blocks are pretty normal. The community has requested showing a larger number of lines at once (suggestions in the range of 30-50 are made there).

~40 lines of code seems reasonable to people using desktop machines with large displays, but could be painful on a phone or tablet. One approach would be to show the larger block size (say, 40 lines) and add a control of some sort that collapses it down to (say) 10 lines (with an "expand" control to put it back). Another approach would be to add a resize control (the usual "drag" control in the lower right) and remember the resulting size for that user in that browser -- i.e. use that size for all future code blocks until the user flushes the cookie. (I assume that would be a cookie.)

I don't think the drag-resize without remembering the preference would be satisfying; the user would have to keep adjusting code blocks, and that seems like it would be annoying. If resize + remember is undesirable, maybe a user preference?

@cellio cellio added area: html/css/js Changes to front-end code complexity: unassessed Needs further developer investigation before complexity/feasibility can be determined. priority: medium type: change request New feature or request labels Mar 1, 2022
@cellio
Copy link
Member Author

cellio commented Mar 23, 2022

Oh, I did create an issue for it at the time -- and it got migrated to the co-design repo: codidact/co-design#43 . I thought I remembered writing that up, but I didn't think to look there.

It sounds like a quick fix would be to change the hard-coded number in co-design (and then qpixel needs to pick up the change). Let's at least do that, and then we can decide if we want to do anything more complex.

@ArtOfCode- ArtOfCode- added complexity: easy Issues that should take limited effort to resolve/fix/build. and removed complexity: unassessed Needs further developer investigation before complexity/feasibility can be determined. labels Mar 24, 2022
@sau226
Copy link
Member

sau226 commented Apr 13, 2022

We've got a fix in codidact/co-design#66 ready to go.

After co-design is shipped with the new value (i.e. PR above is merged and new NPM release made), we just have to bump version in stylesheet lines: app/views/layouts/_head.html.erb and also, for version consistency reasons, app/views/layouts/mailer.html.erb and app/views/layouts/devise_mailer.html.erb).

With the version bump in QPixel being made, the next deploy will close out this issue.

@cellio
Copy link
Member Author

cellio commented Apr 17, 2022

The fix has been merged on the co-design side. We need to finish getting it over to qpixel.

@MoshiKoi
Copy link
Member

What's the status of this? What needs to happen to get the change over to qpixel?

@ArtOfCode-
Copy link
Member

We've got a fix in codidact/co-design#66 ready to go.

After co-design is shipped with the new value (i.e. PR above is merged and new NPM release made), we just have to bump version in stylesheet lines: app/views/layouts/_head.html.erb and also, for version consistency reasons, app/views/layouts/mailer.html.erb and app/views/layouts/devise_mailer.html.erb).

With the version bump in QPixel being made, the next deploy will close out this issue.

Somewhere along the way between this comment and now, someone's bumped the version numbers in those files to 0.12.5. I've just fixed the previous bugs, published and released 0.12.5, so this is now closed with the next deploy.

@cellio
Copy link
Member Author

cellio commented Sep 13, 2022

I just pulled develop and tested this locally and I'm seeing the larger code blocks, yay!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: html/css/js Changes to front-end code complexity: easy Issues that should take limited effort to resolve/fix/build. priority: high type: change request New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants