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.

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

docs: update css import lists #1385

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

p-98
Copy link
Contributor

@p-98 p-98 commented Apr 13, 2024

Problem

I had broken styles after updating from an old version.

Cause

Because I'm using next.js, I have to manually import the stylesheets listed in the documentation. These are outdated.

Solution

I went through all rmwc packages by hand and assured that all files imported by packages/<package>/src/styles.ts are mentioned in the documentation.

I was a little unsure what to do with list-collapsible. The component itself is located within the list directory. There exists collapsible-list.ts and styles.ts containing style imports. However,

  • collapsible-list.ts is a subset of styles.ts,
  • styles.ts imports collapsible-list.ts,
  • none of the three list docs pages mentions collapsible-list.ts.
    I decided to reflect styles.ts as in the other list pages, but you should take a look at this one.

In a personal project I wasn't able to import '@rmwc/list/list-item.css from the list component, so this probably needs correction. (After further investigation it seems these styles end up in styles.css, I just don't know why).

In the snackbar component it reads:

import '@material/button/dist/mdc.button.css';
import '@material/ripple/dist/mdc.ripple.css';

while in all other components the following is used:

import '@rmwc/button/styles';
import '@rmwc/ripple/styles';

I decided to leave this change up to you. But if adapted, the docs need to be updated accordingly.

There is a docs page touch-target.tsx that I was unable to find in the web interface. I still adapted it to reflect the components styles.ts.

Further discussion

Doing this manually took quite some time. Ideally, it would be possible to express the dependencies in a way that docs and code would be generated from it. This would prevent them becoming out of sync, although I don't know if/how this is possible.

Even if it can't be autogenerated, a notice in CONTRIBUTING.md might be added to draw the attention to the fact the docs must be updated when css imports are changed. I would even consider this type of change breaking(!) and mark it as such, since it requires code changes by the user.

Copy link

semanticdiff-com bot commented Apr 13, 2024

Review changes with SemanticDiff.

Analyzed 23 of 23 files.

Overall, the semantic diff is 37% smaller than the GitHub diff.

File Information
Filename Status
✔️ utils/readme/src/readmes/avatar.tsx Analyzed
✔️ utils/readme/src/readmes/button.tsx 61.11% smaller
✔️ utils/readme/src/readmes/card.tsx Analyzed
✔️ utils/readme/src/readmes/checkbox.tsx 66.67% smaller
✔️ utils/readme/src/readmes/chip.tsx 40.0% smaller
✔️ utils/readme/src/readmes/circular-progress.tsx 80.0% smaller
✔️ utils/readme/src/readmes/dialog.tsx Analyzed
✔️ utils/readme/src/readmes/fab.tsx 66.67% smaller
✔️ utils/readme/src/readmes/icon-button.tsx 66.67% smaller
✔️ utils/readme/src/readmes/list-collapsible.tsx 33.33% smaller
✔️ utils/readme/src/readmes/list-variants.tsx 44.44% smaller
✔️ utils/readme/src/readmes/list.tsx 25.56% smaller
✔️ utils/readme/src/readmes/menu.tsx 28.57% smaller
✔️ utils/readme/src/readmes/radio.tsx 66.67% smaller
✔️ utils/readme/src/readmes/ripple.tsx 80.0% smaller
✔️ utils/readme/src/readmes/segmented-button.tsx 76.67% smaller
✔️ utils/readme/src/readmes/select.tsx Analyzed
✔️ utils/readme/src/readmes/snackbar.tsx 66.67% smaller
✔️ utils/readme/src/readmes/switch.tsx 66.67% smaller
✔️ utils/readme/src/readmes/tabs.tsx Analyzed
✔️ utils/readme/src/readmes/textfield.tsx 9.55% smaller
✔️ utils/readme/src/readmes/top-app-bar.tsx 21.6% smaller
✔️ utils/readme/src/readmes/touch-target.tsx Analyzed

Copy link

sonarcloud bot commented Apr 13, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@EmiBemi
Copy link
Member

EmiBemi commented Apr 16, 2024

Thanks for the PR! I think for now we should just let collapsible-list.ts file be. As you said, it's part of the list styles import.

@p-98
Copy link
Contributor Author

p-98 commented Apr 18, 2024

Okay, then we'll leave collapsible-list.ts as is. On my side, two questions are left:

What about list/list-item.css, can you confirm I it ends up as @rmwc/list/styles.css in the production build?

How about the notice in CONTRIBUTING.md? Under "Contributing to Existing Components", something like

- When changes to the style imports are made, the docs in "utils/readme/src/readmes" must be updated. This should be marked as breaking change!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants