comments and expandable changes #2667

2019-07-24T23:11:57+02:00

jessopb commented

2019-07-24 23:11:57 +02:00

(Migrated from github.com)

comments are only expandable if larger than 120 rect.height
expandable checks the useRect ref height before inserting itself

Originally I implemented the useRect in the Comment component but it seems to work contained in Expandable.

For now, I commented out Expandable border-bottom style - I don't think expandable should be in charge of that sort of thing.

comments are only expandable if larger than 120 rect.height expandable checks the useRect ref height before inserting itself Originally I implemented the useRect in the Comment component but it seems to work contained in Expandable. For now, I commented out Expandable border-bottom style - I don't think expandable should be in charge of that sort of thing.

neb-b (Migrated from github.com) reviewed 2019-07-24 23:11:57 +02:00

jessopb (Migrated from github.com) reviewed 2019-07-24 23:40:01 +02:00

src/ui/scss/component/_expandable.scss

jessopb (Migrated from github.com) commented

2019-07-24 23:40:01 +02:00

This will be removed before merge if we agree it doesn't belong.

jessopb (Migrated from github.com) reviewed 2019-07-24 23:42:36 +02:00

src/ui/component/expandable/view.jsx

					
				@ -9,4 +8,4 @@

				type Props = {

				  children: React$Node | Array<React$Node>,

				};

jessopb (Migrated from github.com) commented

2019-07-24 23:42:36 +02:00

This 120 should be a constant, I think. I'm not sure how I'd use that constant for the styles too, though. Base size is 12px and expandable min height is 10rem (=120).

I was considering allowing to pass in as a prop the height at which text becomes expandable as long as it's over 120px

This 120 should be a constant, I think. I'm not sure how I'd use that constant for the styles too, though. Base size is 12px and expandable min height is 10rem (=120). I was considering allowing to pass in as a prop the height at which text becomes expandable as long as it's over 120px

neb-b (Migrated from github.com) reviewed 2019-07-29 16:51:23 +02:00

src/ui/component/expandable/view.jsx

					
				@ -9,4 +8,4 @@

				type Props = {

				  children: React$Node | Array<React$Node>,

				};

neb-b (Migrated from github.com) commented

2019-07-29 16:51:23 +02:00

I just moved it up to the top. I think as long as it's reasonable it's fine. In the browser (and possibly desktop sometime) users can scale the font-size so anything exact falls apart. 120 seems fine.

Sign in to join this conversation.

No reviewers

neb-b