-
Notifications
You must be signed in to change notification settings - Fork 17
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.
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
UI improvements #43
UI improvements #43
Conversation
7df4d62
to
46c7398
Compare
One blocking issue here: EDIT: Well that was easy https://pnpm.io/cli/patch |
@@ -108,9 +108,9 @@ registerRoute(/\/stored\/.*/, async (options) => { | |||
}); | |||
|
|||
registerRoute( | |||
/^https:\/\/cdn.jsdelivr.net\/npm/, | |||
/^https?:\/\/(cdn\.jsdelivr\.net|texlive2\.swiftlatex\.com)/, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm fine with caching them but just make sure you know that in the deployed version, LaTeX dependencies are not served by this cache.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm why so?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here I set the path to be our own website:
ndn-workspace-solid/src/components/share-latex/share-latex/index.tsx
Lines 179 to 182 in 38f1b0f
if (process.env.NODE_ENV && process.env.NODE_ENV !== 'development') { | |
// Only set URL in production mode | |
engine.setTexliveEndpoint(`${location.origin}/stored/`) | |
} |
I think we can have more control if we use OPFS. For example, when we want to support something like loading all packages in a zip file, we will be able to do so. Also, I don't think those files will be updated frequently.
} | ||
|
||
/** PDF Slick component */ | ||
.pdfSlick { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CSS for specific page can be loaded in place:
import cssClassName from './pdfSlick.css'
Then, cssClassName
would be a string containing the correct CSS class name.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting. I do also want to switch to SCSS asap; writing plain CSS is nightmarish
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's also good.
You have already pushed too much to this PR and made it very hard to review. |
Ah yes |
@zjkmxy comments here? EDIT: btw pls squash if you merge; the history is irrelevant. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
WIP
Testing notes: clear vite cache (
node_modules/.vite
) andpnpm install
again.