Skip to content

Conversation

@dmytrovoytko
Copy link

  1. Slimmer popup window header - to leave more space for highlights.
  2. New button "temp Hide" - to temporary hide highlights on page, just reload the page to see them again.
  3. New button "Roll up window" - to see highlights behind popup window.
  4. Copy highlights with '----' separator.
  5. Ability to change highlights order by drag-and-drop - before copying.

1. Slimmer popup window header - to leave more space for highlights
2. New button "temp Hide" - to temporary hide highlights on page, just reload to see them again
3. New button "Roll up window" - to see highlights behind popup window
4. Copy highlights with '----' separator
5. Ability to change highlights order by drag-and-drop - before copying
@dmytrovoytko
Copy link
Author

Hi Jérôme!
Thank you for your great chrome extension, I really like using it, and the structure of your code!
Let me offer you some improvements, I described them above.
I have some more ideas, hope to contribute more!

@dmytrovoytko
Copy link
Author

Hi @jeromepl
Did you have a chance to look, any feedback?

@jeromepl
Copy link
Owner

Hey @dmytrovoytko , thanks for the PR and sorry it's taking me a while to get to it!

Any chance you could split this PR into 5?
It would be much easier to review and approve in that format and I could give feedback on each individual feature

There are some that I would be happy to merge quickly while for some others it would require a bit more discussion and reflexion

@dmytrovoytko
Copy link
Author

dmytrovoytko commented Apr 13, 2023

Hi @jeromepl and thank you for your reply.
It's my first experience like this, agree separate PRs would be easier for you.
Can you please let me know which changes you would like to merge quickly so I could focus on them?

@jeromepl
Copy link
Owner

Hey! I'd say 1,2 and 5 are easier sells than the other two!
For all of them it would also help to include a screenshot of the design.
For 5 I'll have to check if it's worth adding another dependency
For 1 and 2 they have to be not too intrusive (in terms of design) as I don't want to clutter the UI with buttons like this if they won't be used that much

@dmytrovoytko
Copy link
Author

Great!
Sure, here is how it looks:
image

I changed some paddings and made the current color slimmer - more space for highlights.
Button to hide looks quite naturally (IMHO) near original 'Copy all' and 'Delete all'

@dmytrovoytko
Copy link
Author

And here is how "Roll up" works (to see highlights behind popup window)
image
and
image
I think some people find it useful, anyway it's up to you.

@dmytrovoytko
Copy link
Author

And the copied list with separators now looks like this (some users asked about it also, as I remember)
image

@dmytrovoytko
Copy link
Author

Hi Jerome @jeromepl
I created 2 separate PRs for #1 and #2.
Please let me know if you have any questions

@dmytrovoytko
Copy link
Author

Hi Jerome @jeromepl !

Did you have a chance to look?

@dmytrovoytko
Copy link
Author

dmytrovoytko commented Jun 1, 2023

Hi Jerome @jeromepl ! How are you doing?
It's been a while. Do you have a little time and interest for this?
(2 separate PRs are easy to check)

@jeromepl
Copy link
Owner

Sorry for the long delay! I have limited time I can put on this project at the moment unfortunately.
I took a quick look at the 2 PRs and left some comments there.

Looking at the other features you have ready here, I have a couple of thoughts:

  • Why do you think a "Roll up" button would be useful? Could users just use the chrome toolbar icon of the Highlighter extension instead?
  • For the copied list with separators, why do you prefer a separator like "----" over a newline "\n"? Could this be fixed by adding extra spacing between highlights (like 2 newlines instead of 1)?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants