Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Platform-independent frontend slots #940

Draft
wants to merge 55 commits into
base: master
Choose a base branch
from
Draft

Platform-independent frontend slots #940

wants to merge 55 commits into from

Conversation

eritbh
Copy link
Member

@eritbh eritbh commented May 14, 2024 •
edited
Loading

Basically TBListener but beefier - a proper solution for defining platform-independent locations where UI elements can be placed by modules.

frontend/index.tsx defines a set of locations where modules might want to place buttons, regardless of platform. It also defines a standardized set of context information available to each location. It exposes a function modules can use to have React components rendered at those locations, and it exposes types used by platform-specific observers to set where each location is rendered within that specific frontend.

frontend/newreddit.tsx defines the observer for new Reddit. It's the simplest to implement by far, because the elements are mostly already provided by the new Reddit jsAPI; we just insert new renderers into the slots we receive. Observers for other platforms are stubs for now but will be implemented before this PR is ready to merge (with the exception of shreddit, which will happen in its own PR).

The end goal here is to replace TBListener with this new system. TBListener handles new Reddit via jsAPI only, and old Reddit is basically handled by spoofing TBListener events so that we can pretend old Reddit has jsAPI. Extending this system to Shreddit presents challenges, because we need an entirely new system to scrape the required information from the shreddit frontend before emitting it as events, and the actual data models provided by jsAPI are not super well defined.

Fixes #865. Shreddit observer will be handled separately, after this PR, as a fix for #864.

  • Implement locations
    • By submission author names
    • By comment author names
    • By modmail message author names
    • User hovercards
      • Partially done, implemented for the modmail sidebar and nothing else
    • Bottom of submissions
    • Bottom of comments
  • Implement observers
    • Implement new Reddit observer
    • Implement old Reddit observer
    • Implement modmail observer
    • Set up placeholder for shreddit
      • Actually implementing the Shreddit observer will happen in a future PR. This PR is just a refactor of existing functionality into a system that will make the eventual Shreddit implementation possible.
  • Rework modules to use this API instead of TBListener
    • As usual this has started with modnotes since it's the freshest code. Other observers should be implemented before doing more of this that's done so I can do this now
  • Remove TBListener and the oldreddit module after verifying nothing is left that relies on it

@eritbh eritbh added this to the v7 milestone May 14, 2024
dependabot bot and others added 18 commits August 5, 2024 18:14
@dependabot @eritbh
Bump webextension-polyfill from 0.10.0 to 0.12.0 (#942)
f94b21d 
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@dependabot @eritbh
Bump react-dom and @types/react-dom (#945)
5fc0ad1 
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@dependabot @eritbh
Bump dprint from 0.45.0 to 0.46.2 (#950)
6ba2850 
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@dependabot @eritbh
Bump @rollup/plugin-commonjs from 25.0.7 to 26.0.1 (#948)
1652c52 
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@dependabot @eritbh
Bump braces from 3.0.2 to 3.0.3 (#951)
645892d 
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@dependabot @eritbh
Bump inquirer from 9.2.15 to 9.3.2 (#955)
182cee6 
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@dependabot @eritbh
Bump dprint from 0.46.2 to 0.47.0 (#956)
396e71e 
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@dependabot @eritbh
Bump typescript from 5.4.3 to 5.5.3 (#957)
c92cf47 
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@dependabot @eritbh
Bump iter-ops from 3.1.1 to 3.2.0 (#958)
6ec88fb 
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@dependabot @eritbh
Bump framer-motion from 11.2.2 to 11.3.2 (#962)
b9c6025 
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@dependabot @eritbh
Bump inquirer from 9.3.2 to 10.0.1 (#960)
2c5a268 
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@eritbh
Fix macros in modmail (#963)
86e1b3c
@eritbh
More release meta fixes (#965)
ba20f72
@eritbh
v7.0.0-beta.4
3aaa711
@eritbh
firefox is so picky about its version strings
01712b0
@eritbh
v7.0.0-beta.5
8b6b5df
@eritbh
please accept this string i am begging you
cd54af4
@eritbh
v7.0.0-beta.6
820294e
eritbh
eritbh commented Aug 5, 2024
View reviewed changes
extension/data/frontends/index.tsx
// returns React content to be rendered in the slot
export type PlatformSlotContent<Location extends keyof PlatformSlotDetails> = ComponentType<{
details: PlatformSlotDetails[Location];
location: Location;
Copy link
Member Author

Choose a reason for hiding this comment

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

location is a global name why did i think it was a good idea to name a parameter that. call it slotName or something

@eritbh
Copy link
Member Author

eritbh commented Oct 17, 2024

Wanted to do some work on this, but because slot contents are rendered in the shadow DOM now, legacy styles don't work with this PR at all. Putting a JQueryRenderer in a slot works but there's no styles applied to its contents because of the shadow DOM boundary.

I've gotten this up-to-date with current master again but I think I'm going to continue not working on this until all the slottable UI stuff is reworked to use component styles. We can maybe get away with not doing literally everything first, since windows and stuff we can just insert into the bottom of <body> outside the shadow DOM or whatever, but I'm not confident about that. In any case it's clear that the place to start is with the rest of the styles being redone

@eritbh
Copy link
Member Author

eritbh commented Oct 17, 2024 •
edited
Loading

okay i lied i got a little bit of shreddit working, enough to show that this will actually work there in the future. the focus here should still not be on that yet though - we still need parity on all the other platforms before i can put a ton of effort into the shreddit observer. i just wanted to throw together a proof of concept that assures me all this work isn't for nothing

branch compare with a partially working shreddit observer: compare feat/platform-observers...feat/shreddit-observer as of 2024-10-17

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
code quality core: tblistener enhancement New feature or request
Projects
None yet
Milestone
v7
Development

Successfully merging this pull request may close these issues.

Long-term solution for generalizing UI placement across frontends
1 participant
@eritbh