Skip to content

Conversation

@yakov116
Copy link
Member

@yakov116 yakov116 commented Oct 30, 2020

@yakov116
Copy link
Member Author

yakov116 commented Nov 1, 2020

Video_2020-10-31_230407

Feedback needed.

How's this?

Does not make sense to show a screenshot of the feature your looking at
@yakov116 yakov116 marked this pull request as ready for review November 1, 2020 04:02
@fregante
Copy link
Member

fregante commented Nov 1, 2020

A few suggestions:

  • I’d move it to the right side like in my mock-up, it’s easy to implement via flex
  • I’d just embed the image as suggested, as a square thumbnail with object-fit: cover and perhaps a light border unless we just use a blueish background for the whole box, which
  • Clicking the thumbnail would open it exactly like it happens for every other markdown image on the site: raw, without lightbox

@yakov116
Copy link
Member Author

yakov116 commented Nov 1, 2020

But many screenshots would look funny in that size, No?

I will give it a shot later (I happen to like the way it is JMHO)

@fregante
Copy link
Member

fregante commented Nov 1, 2020

But many screenshots would look funny in that size, No?

It's just a preview, equivalent to a button: 100x100px-ish image on the left and description on the right

@yakov116
Copy link
Member Author

yakov116 commented Nov 1, 2020

Can you help me with the flex (I have never used flex before)?

@fregante
Copy link
Member

fregante commented Nov 1, 2020

You probably just need:

wrapAll(<div className="d-flex"/>, [leftHandCommitInfo, yourNewBox])

And yourNewBox itself can just be another <div className="d-flex"/>

@yakov116
Copy link
Member Author

yakov116 commented Nov 2, 2020

Sorry was unable to do it last night. 🤒 since last night will try later to do it.

@fregante
Copy link
Member

fregante commented Nov 3, 2020

By the way, either this isn't ready, or it doesn't fix #3630 since it doesn't include the other pieces of information suggested

@fregante fregante marked this pull request as draft November 3, 2020 06:34
@yakov116
Copy link
Member Author

yakov116 commented Nov 5, 2020

I am going to allow anyone to take over this PR. I added the api call and filtered it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

Meta-feature: Add information box to each feature file

2 participants