sphil icon indicating copy to clipboard operation
sphil copied to clipboard

feat: The prompt to edit the current page on GitHub now triggers a contextual modal component

Open PatrickLarocque opened this issue 1 year ago • 6 comments

PR Author's Note

closes #80

Checklist

  • [ ] Philosophical or literary contribution (docs). Leave unchecked for code contribution.

    • IMPLIED CONSENT By opening this pull request and contributing philosophical or literary content, I accept that my writing is submitted under the ATTRIBUTION-NONCOMMERCIAL-SHAREALIKE 4.0 INTERNATIONAL, which:

      • Prohibits commercial reuse of the content.
      • Allows sharing, remixing, and building upon the material as long as attribution is given.

      I understand that my writing may be modified, remixed, and built upon by others within the systemphil/sphil or sPhil project, in accordance with the license terms, indefinitely. See legal code.

    • REQUIRED I have followed the formatting guidelines and verified there are no formatting bugs. Try markdown preview here.

    • REQUIRED I have followed the Chicago author-date style.

    • REQUIRED I have added or verified metadata title, description, and contributors at the very top of the file followed by a ## title heading. Additionally, I have ensured isArticle is set to true. Example:

      ---
      title: The Immediate Difference Between Pure Being and Pure Nothing
      description:
          Learn about the difference between being and nothing in Hegel's
          Science of Logic.
      isArticle: true
      authors: Jerry Maguire (2024)
      editors: Steve Stevenson (2023), Karen Hansen (2022)
      contributors:
      ---
      
      ## My Article Title
      
      Further information

      I have signed the document with my name/username under either as Authors, Editors or Contributors.

      Use Authors if you have created and substantially added content.
      Use Editor if you have made substantial edits or review.
      Use Contributor if you have made minor edits, reviews or contributions.
      If you've done multiple, pick the most weighted: Author > Editor > Contributor.
      If you prefer to remain anonymous, that's fine too, but note that a record of your contributions based on your GitHub username will exist here in the codebase.

    • REQUIRED I have ensured that the project's central bibliography contains the necessary bibliographical details for the citations I have used.

    • Optional My article is a stub or I want to actively encourage contribution, I've added the Stub component to the bottom of my content or where relevant:

      import Stub from "@/components/Stub";
      
      <Stub />;
      
  • If Docs contribution is unchecked: Code contribution (Apache version 2 license)

    All code apart of what is inside src/pages/** (excluding /contributing/**, _app.mdx, _document.tsx, _meta.json, acknowledgements.mdx, index.mdx, privacy.mdx, team.mdx, terms.mdx) is subject to Apache version 2 license. Basically, anything outside of content, literature, philosophy.

PatrickLarocque avatar Sep 30 '24 01:09 PatrickLarocque

I used shadcn/ui for the component I made. At the moment, it doesn't play very nicely with the currenting themeing solution along w/ MUI and the other component libraries. However, given this is a next project, I feel like Shad/cn might is worth considering, given that it is maintained and supported by vercel. Although some refactoring would be needed to port existing components and theming. This is of course an open discussion and I could certainly attempt to rewrite this in MUI or HeadlessUI, please let me know.

I've provided a link to the shad/cn docs in case it is something you have not considered: https://ui.shadcn.com/

image image

Please note that I also included a bump to Next 14, which is likely a hasty addition and can be removed if desired.

PatrickLarocque avatar Sep 30 '24 01:09 PatrickLarocque

Awesome stuff, Pat! Just checking out the branch now.

  1. I'm open to new UI/UX additions, but my question is maintainability and compatibility. Can shadcn eventually play nice with our setup? Would it be issues downstream if we have multiple UI libraries? Perhaps the actual look might become an issue. I personally favor sticking with one system as far as possible, and we mainly use Nextra's UI stuff with some minor additions from MUI and other things, but shadcn might be a nice addition.
  2. The link should change cursor to a hand like the other links in the vicinity.
  3. There is a slight pop when the modal opens where the entire page shifts due to the scrollbar disappearing.
  4. Not sure about adding an extra icon library when we already have MUI icons and it includes github icon.
  5. Next wasn't updated because Nextra runs on the older version (pages router). It might still be compatible but I just hadn't checked and the security features aren't are problem here.

Will try it some more later on and let you know if there's anything else, but this is a solid start!

Firgrep avatar Sep 30 '24 12:09 Firgrep

Thanks for the feedback! I will be able to address most of your comments after my day job. Will get back to you shortly with some fixes.

PatrickLarocque avatar Sep 30 '24 13:09 PatrickLarocque

Something you might consider is the MUI modal. Seems to handle the popping issue and comes with a nice transition effect. https://mui.com/material-ui/react-modal/#transitions

Firgrep avatar Oct 01 '24 08:10 Firgrep

@Firgrep Speaking from experience, using MUI can be a deal with the devil. MUI components offer great out-of-the-box functionality at the expense of customizability. They can be very difficult to customize depending on the component. If we wanted to change the look/feel of the app down the road, MUI could bite us in the behind.

From what I know of shad/cn, on the other hand, their components seem very customizable and transparent to developers. Of course, I've never used that library before, so @PatrickLarocque would be a better person to ask about that.

It might not be an issue in this case, however. Just a cautionary tale I wanted to bring up because I've been burned by MUI before.

JMielbrecht avatar Oct 01 '24 15:10 JMielbrecht

Points taken and appreciated @JMielbrecht . While I agree that MUI components tend to be very finished, I've never had an issue with customizability (and we've been using it daily for a frontend at work for over a year). The sx prop virtually allows one to change any CSS. Of course, MUI comes with a certain look, but that is something we needn't follow, and we needn't use it always either. I just figured they have a very polished modal functionality we could use and inside of which we put in whatever.

I'm hesitant about shadcn because of (1) can the problem be adequately solved with existing tools? And (2) I'm unsure about the API and maintainability of external code that's copy-pasted directly rather than imported as a package. What if the components have a bug? Do we re-copy-paste individual bits, rather than just update a version of a package? But I'm open to hear more about this.

Firgrep avatar Oct 01 '24 21:10 Firgrep

Closed due to going stale/no activity.

Firgrep avatar Nov 15 '24 08:11 Firgrep