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

feat: wrap text, align text vertically and auto resize row #1406

Draft
wants to merge 43 commits into
base: main
Choose a base branch
from

Conversation

AyushAgrawal-A2
Copy link
Contributor

@AyushAgrawal-A2 AyushAgrawal-A2 commented Jun 4, 2024

closes #1227
closes #1460

Changes:

  • Cell Label - Wrap, Clip & Overflow
  • Cell Label - Vertical Align
  • Inline Editor - Same text rendering as cell format (horizontal align, vertical align and wrapping)
  • Re-wrap text on cell width change
  • Auto row resize for wrapped text - on cell width change or content change
  • Double click on row heading to auto adjust row height
  • Fixed bugs related to neighbouring hash position and clipping
  • Update clipboard for vertical align
  • Multiplayer sync after transient / auto resize
  • Store manual / auto resize state. Disable auto resize for manually resized rows.
  • Auto resize column (on column heading dbl click) should resize to unwrapped text width.
  • Tests

@cla-bot cla-bot bot added the cla-signed label Jun 4, 2024
Copy link

vercel bot commented Jun 4, 2024

@AyushAgrawal-A2 is attempting to deploy a commit to the Quadratic Team on Vercel.

A member of the Team first needs to authorize it.

@AyushAgrawal-A2 AyushAgrawal-A2 marked this pull request as ready for review June 9, 2024 03:38
@AyushAgrawal-A2
Copy link
Contributor Author

@luke-quadratic @davidfig @ddimaria @davidkircos
This is ready for initial feedback. I am not sure how far I am from desired functionality.

@AyushAgrawal-A2
Copy link
Contributor Author

@davidfig I wanted this also, but I was not sure about implementation. This requires a flag in schema to denote whether a row height is manually set or set by auto resize. I was unsure if I should add this boolean flag in schema or I can reuse format_rows in your running PR. Or maybe something else...? Can you please suggest schema entry for this ?

Can't you rely on undefined for row height? Once it has a value, then use that instead. Or am I missing something?

@davidfig
Auto row resizing also sets a value in SheetsOffsets and this value propagates to client and renderer ... so value is defined in both cases whether manual resize or auto.

@davidfig
Copy link
Collaborator

davidfig commented Jun 10, 2024

Auto row resizing also sets a value in SheetsOffsets and this value propagates to client and renderer ... so value is defined in both cases whether manual resize or auto.

Ah, yes, you're right.

Maybe we add an auto-resize entry (which is how I think Excel does it). The easiest way would be to add an auto_size: BTreeMap<i64, f64> to Offsets. The f64 is the current size for that row/column (but changeable based on the actual content for that column/row). We then remove that entry if we manually change the size (which is stored in sizes). Alternatively, we can create an enum for sizes for size and autosize. Either way should work.

It's tricky since we have to update that autosize whenever content is changed, but we only know the wrapped size after we render it. Ideally, columns would also work this way.

This would be a bunch of work. Feel free to punt on it, and we can work toward merging this PR, and open an issue to do the autosize down the road.

@AyushAgrawal-A2
Copy link
Contributor Author

This would be a bunch of work. Feel free to punt on it, and we can work toward merging this PR, and open an issue to do the autosize down the road.

@davidfig
I would try to finish this in this pr itself. I wanted to ask the schema doubt as you guy might have already planned for this. I didn't want to add random entries to schema myself.
This pr will anyways need to be refactored to incorporate changes in your Select all, column, and row #1363 pr, after it is merged in main.

@AyushAgrawal-A2
Copy link
Contributor Author

This looks super nice. A few things I noticed on the functionality:

  • there's a noticeable difference between the rendered text vs Monaco when wrapping. Probably has to do with the CSS vertical spacing
  • the renderer is dropping characters when wrapping the text, mostly on the last line of the text
  • it's auto adjusting the height after a user manually adjusted it. Once the height is changed, it should not auto-adjust unless the user double-clicks on the corner of the row heading. It should clip the text if too big instead
  • the down arrow on the floating context menu is a bit large (I really like this approach)

@davidfig
all done

@AyushAgrawal-A2 AyushAgrawal-A2 marked this pull request as draft June 28, 2024 18:23
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.

bug: resizing columns cuts chunks of words off as you resize Resize rows and wrap text vertically
3 participants