hugo-cite icon indicating copy to clipboard operation
hugo-cite copied to clipboard

Should I contribute or fork?

Open xbc5 opened this issue 2 years ago • 13 comments

I have some issues with the current implementation that I'd like to change. My question is whether these are suitable for a PR, or whether I should maintain my own fork. What features would you accept, and what would you not? I can submit the ones that you would, and fork after those.

  1. it doesn't throw for invalid cite key -- it should, there is never a case where an inline message is the right answer:
    • [ ] feat: errorf when a cite-key is not found;
  2. in-text citation can linebreak on ( : e.g. (\nfoobar):
    • [ ] feat: perhaps use some HTML markers (linebreak hints) to indicate that it shouldn't break there if at all possible;
  3. the popup lacks a cite element (#20):
    • [ ] feat: use a cite element on the source title within the popup;
  4. the popup authors could be wrapped in a span for easier styling (e.g. boldface);
    • [ ] feat: wrap it in a span, and give it a class name;
  5. all elements, including sub elements could have classes for easier custom styling;
    • [ ] feat: give every major element a class name;
  6. there is a space after the citation: (Foo, 2009) .
    • [ ] fix: remove the space;
  7. the section tag (bibliography) doesn't have a heading;
    • [ ] feat: give it a heading;
  8. should the bib be part of the main content (shortcode) or outside of it (partial) as an "aside"?
    • [ ] feat: implement the usage of a partial (if possible);
  9. the popup doesn't have a configurable product card. Sometimes it's suitable to include a product card to a book on Amazon for example via an embedded widget (iframe);
    • [ ] feat: create a configurable lookup table in the main config that pairs a cite-key to a partial;
  10. the definition list doesn't contain a product card
    • [ ] feat: same as the last feature, but for the definition list too;
  11. the reference for books doesn't embed the ISBN, only the URL, which isn't always useful
    • [ ] feat: a configuration option to embed either or both;
  12. :hover doesn't work on mobile, and if feature 9 exists, then clicking the reference won't show a popup
    • [ ] feat: on small screens, when a citation is clicked, the popup is show with a "full ref" anchor linking to the bibliography; on larger screens hover works as normal. This may require a small amount of JavaScript, although a solution via the CSS :target is possible though setting the URL fragment (although not ideal, and a bit of a hack);
  13. proper citations for videos -- e.g time codes (#50)
    • [ ] feat: include time codes in references
  14. anchors are draggable, is this necessary; does anyone use this default HTML feature?
    • [ ] feat: make anchors undraggable
  15. quotations (blockquotes) are hard to style within shorcodes -- typcially .Inner is the text, but what about the citation? If an author is able to write their own blockquote shortcode, how is it then possible to set a citation? It cannot be passed via parameters, and .Inner is occupied.
    • [ ] feat: create a blockquote shortcode -- allow author style overrides; accept a key as a parameter and embed a citation;
  16. The bibliography causes overflow on small screens: the anchor does not break when there's long words
    • [ ] fix: for tiny screens set word-break: break-all, for smallish screens set word-break: unset
  17. The dd in the bibliography definition list has a margin too large for small screens:
    • [ ] fix: set dd margin-inline-start: clamp(0px, 5vw, 40px); (40px is the original margin value);
  18. the popover (.hugo-cite-citation) causes horizontal overflow, even when it's not visible (particularly on small screens) (#18):
    • [ ] fix: set width of popover to 0; on ::hover, set width to auto;
  19. references should be an aside (because it's indirectly related to the main content)
    • [ ] refactor: change references section to aside;
  20. some characters should be HTML character entities -- e.g. "&" => &;
    • [ ] possibly scan each line and replace the relevant characters; or consider updating the documentation to ask users to use HTML character entities where necessary; or both (scanning for common characters like & or (c));

What are your thoughts?

xbc5 avatar Oct 02 '22 18:10 xbc5

Please make a PR.

On Sun, Oct 2, 2022 at 11:17 AM xbc5 @.***> wrote:

I have some issues with the current implementation that I will fix. My question is whether these are suitable for a PR, or whether I should maintain my own fork.

  1. it doesn't throw for invalid cite key -- it should
  • in-text citation can linebreak on ( : (\nfoobar) -- perhaps use some HTML markers (linebreak hints) to indicate that it shouldn't break there;

  • the popup, dl, and/or in-text citation could be a cite element;

  • the popup tile could be wrapped in a span for easier styling;

  • all elements, including sub elements could have classes for easier styling;

  • there is a space after the citation: (Foo, 2009) .

  • the section tag (bibliography) doesn't have a heading;

  • should the bib be part of the main content (shortcode) or outside of it (partial) as an "aside"?

  • make proper use the the element;

— Reply to this email directly, view it on GitHub https://github.com/loup-brun/hugo-cite/issues/62, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAJ2JS6HFZSLSHDCHZY26DWBHGSTANCNFSM6AAAAAAQ3ADMT4 . You are receiving this because you are subscribed to this thread.Message ID: @.***>

-- All the best, -Hugh

Sent from my iPhone

HughP avatar Oct 02 '22 23:10 HughP

That was the first one I thought of. Any else?

What do you think @loup-brun ?

xbc5 avatar Oct 04 '22 04:10 xbc5

All PRs accepted! Sorry I don’t have much time to maintain this.

loup-brun avatar Oct 04 '22 13:10 loup-brun

No problem. I will get started soon. I will create separate issues for each before I submit a PR. I'd welcome some feedback on them.

Thanks!

xbc5 avatar Oct 04 '22 17:10 xbc5

given the size of your list, please make several PRs... preferably one each per "feature"... it allows smoother approval and integration.

On Mon, Oct 3, 2022 at 9:51 PM xbc5 @.***> wrote:

That was the first one I thought of. Any else?

What do you think @loup-brun https://github.com/loup-brun ?

— Reply to this email directly, view it on GitHub https://github.com/loup-brun/hugo-cite/issues/62#issuecomment-1266395874, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAJ2JT7FPOC5NESMZHUDEDWBOZUPANCNFSM6AAAAAAQ3ADMT4 . You are receiving this because you commented.Message ID: @.***>

HughP avatar Oct 11 '22 08:10 HughP

given the size of your list, please make several PRs... preferably one each per "feature"... it allows smoother approval and integration.

Absolutely.

I haven't yet started working on this because it's quite low down on my current list of priorities. Once I complete the core features for my website I will start work on this. Right now I am adding to the list, feel free to provide feedback.

Thanks.

xbc5 avatar Oct 19 '22 04:10 xbc5

Hi @xbc5 !

Thanks for collecting all of these tasks!

Firstly, I'd like to state that I support @HughP 's suggestion to split this up into several PRs.

Since I'm currently also working on some of the tasks you collected in this issue, for reducing redundancy:

Is there any task that you have already engaged in? If so, would you mind creating draft PRs for to indicate which of the tasks you're on? That would be great and allow me to proceed! Thank you!

k127 avatar Nov 28 '22 19:11 k127

there is a space after the citation: (Foo, 2009) .

I fixed this in #64.

k127 avatar Nov 28 '22 19:11 k127

Hi @xbc5 !

Thanks for collecting all of these tasks!

Firstly, I'd like to state that I support @HughP 's suggestion to split this up into several PRs.

Since I'm currently also working on some of the tasks you collected in this issue, for reducing redundancy:

Is there any task that you have already engaged in? If so, would you mind creating draft PRs for to indicate which of the tasks you're on? That would be great and allow me to proceed! Thank you!

I haven't started any yet, and likely won't for a number of weeks. I'll mention you when I do, and yes I will open PRs early and push as I'm working on them.

Also, mention this issue in your PRs, and I will check the tasks off when they're merged.

NOTE: that I'd like to reserve 9. and 10. (product cards) for myself, because I have some specific ideas, and those two issues were the inspiration for me wanting to contribute. Thanks.

xbc5 avatar Nov 28 '22 20:11 xbc5

Just an update: I still intend to start this in the not too distant future.

xbc5 avatar Jan 14 '23 11:01 xbc5

I am having second thoughts about issuing PRs if the maintainer isn't active. @loup-brun any chance you can share responsibilities? I wouldn't mind doing it, but if you have someone else in mind..

xbc5 avatar Jan 16 '23 16:01 xbc5

I’m sorry I was unavailable to maintain this project, I would like to circle back in the coming weeks. @xbc5 I’ve invited you as a collaborator, so you can review PRs. Thank you for you time!

loup-brun avatar Jan 16 '23 17:01 loup-brun

Accepted, thanks. I'll need to get familiar with the code before reviewing PRs. I'm almost finished my prior engagements, with two small issues left (on my own, separate project). After I have done those I will tick off a couple of these issues to get myself familiar.

xbc5 avatar Jan 16 '23 17:01 xbc5