Open
Conversation
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: timea-solid <4144203+timea-solid@users.noreply.github.com>
Co-authored-by: timea-solid <4144203+timea-solid@users.noreply.github.com>
[WIP] Address feedback on local dev setup pull request
[WIP] Update local dev setup based on review feedback
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request modernizes the contacts-pane codebase by extracting inline styles into CSS files for better maintainability and accessibility, consolidating RDF form definitions, and introducing a webpack-based build system with a development server. The changes also remove deprecated autocomplete functionality related to public data queries.
Changes:
- Extracted all inline styles to dedicated CSS files with proper scoping under
.contactPaneclass - Consolidated RDF forms from multiple files into a single
individualAndOrganizationForm.ttlfile - Replaced Babel-only build with webpack bundling and added development server infrastructure
- Removed public data autocomplete features (publicData.ts, autocompletePicker.ts, autocompleteBar.ts, autocompleteField.ts)
- Replaced heavy mime-types package (~170 KiB) with lightweight inline MIME type helpers
Reviewed changes
Copilot reviewed 36 out of 40 changed files in this pull request and generated 11 comments.
Show a summary per file
| File | Description |
|---|---|
| webpack.dev.config.mjs | New webpack dev server configuration with HtmlWebpackPlugin and polyfills |
| webpack.config.mjs | New webpack production build configuration for both minified and unminified bundles |
| package.json | Updated build scripts to use webpack, removed mime-types dependency, added webpack dev dependencies |
| babel.config.mjs | Removed .sparql from inline import extensions |
| jest.config.mjs | Added CSS module mock mapping for tests |
| src/ontology/individualAndOrganizationForm.ttl | Consolidated form definitions for individuals and organizations |
| src/ontology/forms.ttl | Deleted (consolidated into individualAndOrganizationForm.ttl) |
| src/ontology/organizationForm.ttl | Deleted (consolidated into individualAndOrganizationForm.ttl) |
| src/rdfFormsHelper.js | New helper module for loading and rendering RDF forms |
| src/styles/*.css | Five new CSS files extracting inline styles with proper scoping |
| src/contactsPane.js | Replaced inline styles with CSS classes, fixed typo "an contact" → "a contact" |
| src/webidControl.js | Replaced inline styles with CSS classes, updated to use solid-ui's renderAutocompleteControl |
| src/toolsPane.js | Replaced inline styles with CSS classes |
| src/individual.js | Refactored form loading to use rdfFormsHelper, replaced inline styles with CSS classes |
| src/mugshotGallery.js | Replaced mime-types package with lightweight inline MIME helpers |
| src/mintNewAddressBook.js | Replaced inline styles with CSS classes |
| src/groupMembershipControl.js | Removed commented-out imports |
| src/publicData.ts | Deleted (autocomplete functionality removed) |
| src/autocompletePicker.ts | Deleted (replaced by solid-ui implementation) |
| src/autocompleteField.ts | Deleted (replaced by solid-ui implementation) |
| src/autocompleteBar.ts | Deleted (replaced by solid-ui implementation) |
| src/instituteDetailsQuery.sparql | Deleted (no longer needed) |
| dev/* | New development environment with HTML template, context setup, and global CSS |
| test/mocks/styleMock.js | New Jest mock for CSS imports |
| declarations.d.ts | Updated to declare .css modules instead of .sparql |
| README.md | Added comprehensive documentation for dev server, testing, and build process |
| test/unit/data-reformat-test.test.ts | Updated test expectation from 'contact' to 'Contact' for consistency |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/styles/toolsPane.css
Outdated
| .contactPane .toolsTable { | ||
| font-size: 120%; | ||
| margin: var(--spacing-md); | ||
| border: 0.1em var(--color-border-pale); |
There was a problem hiding this comment.
The border CSS property is incomplete. It should specify either a style (e.g., "solid") or the full declaration should be "border-color: var(--color-border-pale)". The current syntax "border: 0.1em var(--color-border-pale);" is missing the border style and will not render correctly.
webpack.dev.config.mjs
Outdated
| use: ['babel-loader'], | ||
| }, | ||
| { | ||
| test: /\.ttl$/, // Target text files |
There was a problem hiding this comment.
Comment has extra space: "Target text files" should be "Target text files"
package.json
Outdated
| "test": "jest --no-coverage", | ||
| "test-coverage": "jest --coverage", | ||
| "prepublishOnly": "npm run build && npm run lint && npm run test", | ||
| "preversion": "npm run lint && npm run typecheck && npm run test", |
There was a problem hiding this comment.
The preversion script references 'typecheck' but this script has been removed from package.json. This will cause the preversion script to fail.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I started by consolidating the RDF forms used.