Skip to content
This repository was archived by the owner on Jun 21, 2023. It is now read-only.

Conversation

@StanleyGoldman
Copy link
Contributor

@StanleyGoldman StanleyGoldman commented Nov 26, 2018

This adds functionality to display a different inline glyph in different situations.

  1. When there is a comment
  2. When there is an annotation
  3. Both

@StanleyGoldman StanleyGoldman changed the title Using different inline comment & annotation markers [WIP] Using different inline comment & annotation markers Nov 26, 2018
@donokuda donokuda mentioned this pull request Nov 27, 2018
2 tasks
@StanleyGoldman StanleyGoldman changed the base branch from features/check-suite-annotations to master November 27, 2018 12:30
@donokuda
Copy link
Contributor

Here's where I've landed on this one:

When there is a comment

screen shot 2018-11-28 at 11 34 16 am

When there is an annotation

screen shot 2018-11-28 at 11 33 32 am

Both

screen shot 2018-11-28 at 11 33 02 am

I landed on using rectangle since it's close to how inline annotations look like and being able to stack them vertically helped communicate that there are "more than one" types of annotations and comments. We could consider utilizing more colors to indicate the type of annotation (instead of me sticking with one color for all annotations.)

@donokuda
Copy link
Contributor

donokuda commented Nov 28, 2018

Pushed a few commits that tries a different approach to markers

When there is a comment

screen shot 2018-11-28 at 3 11 13 pm

When there is an annotation

screen shot 2018-11-28 at 3 13 01 pm

Both

screen shot 2018-11-28 at 3 12 03 pm

@donokuda
Copy link
Contributor

donokuda commented Nov 29, 2018

Pushed a commit that updates the colors of the annotation markers

screen shot 2018-11-29 at 11 40 35 am

screen shot 2018-11-29 at 11 32 27 am

screen shot 2018-11-29 at 11 32 10 am

@StanleyGoldman I'm going to remove my assignment, but let me know if I should take a look at anything else.

@donokuda donokuda removed their assignment Nov 29, 2018
@StanleyGoldman StanleyGoldman changed the title [WIP] Using different inline comment & annotation markers Using different inline comment & annotation markers Nov 30, 2018
@StanleyGoldman StanleyGoldman removed their assignment Nov 30, 2018
Copy link
Contributor

@grokys grokys left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM - I really like how this looks!

@StanleyGoldman StanleyGoldman merged commit 2b46a3c into master Nov 30, 2018
@StanleyGoldman StanleyGoldman deleted the features/check-suite-annotations-inline-markers branch November 30, 2018 14:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants