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

chore: remove component usage files #9052

Merged
merged 19 commits into from
Jul 11, 2024
Merged

Conversation

DitwanP
Copy link
Contributor

@DitwanP DitwanP commented Apr 3, 2024

Related Issue: #9015

Summary

Removing usage sample files from repo since we now have links to each components page on the doc site for more in depth information and samples.

@github-actions github-actions bot added the chore Issues with changes that don't modify src or test files. label Apr 3, 2024
@DitwanP DitwanP marked this pull request as ready for review April 8, 2024 18:39
@DitwanP DitwanP requested a review from a team as a code owner April 8, 2024 18:39
@driskull
Copy link
Member

driskull commented Apr 8, 2024

I'm not sure I agree with removing these at this time.

@jcfranco what about where we are lacking coverage for some of these on the doc site? For example, there is no virtual element info on popover in the doc site.

If we remove some of these usage files I'm concerned we will be losing info on how to do some things.

IMO it would be better to leave these until we can assure we have the same coverage of information on the doc site.

Since Stencil automatically uses these files an generates documentation samples, it would be nice if we could keep these and use them to generate some of the doc site samples. That way, the samples are within the code base and can easily be referenced. A developer who is developing a component or functionality, has the ability to define usage easier if they can do so within the code base.

@DitwanP
Copy link
Contributor Author

DitwanP commented Apr 8, 2024

I'm not sure I agree with removing these at this time.

@jcfranco what about where we are lacking coverage for some of these on the doc site? For example, there is no virtual element info on popover in the doc site.

If we remove some of these usage files I'm concerned we will be losing info on how to do some things.

IMO it would be better to leave these until we can assure we have the same coverage of information on the doc site.

So the plan was to go through the components on the doc site and improve coverage, fix descriptions, and make sure things are still accurate.

Maybe we just leave this for now until the doc site work is done, which is what I was thinking to do initially but wasn't sure if it mattered that much.

Since Stencil automatically uses these files an generates documentation samples, it would be nice if we could keep these and use them to generate some of the doc site samples. That way, the samples are within the code base and can easily be referenced. A developer who is developing a component or functionality, has the ability to define usage easier if they can do so within the code base.

I'm uncertain about the advantages here. It seems like the documentation site offers more detailed examples that often encompass most usage scenarios. While the codebase contains numerous usage files with minor differences, such as adding a property or two to a component, the documentation site could likely achieve the same effect with a toggle.

@DitwanP DitwanP marked this pull request as draft April 8, 2024 21:20
@jcfranco
Copy link
Member

jcfranco commented Apr 8, 2024

what about where we are lacking coverage for some of these on the doc site? For example, there is no virtual element info on popover in the doc site.

Good point on removing usage files that don't have a counter part in the doc site. Can we come up with a list of useful usage files that are missing from the doc site? I have a hunch it will be a small list. In any case, we can prioritize those items for future doc improvements.

Maybe we just leave this for now until the doc site work is done, which is what I was thinking to do initially but wasn't sure if it mattered that much.

This seems like the best path forward. Once we know we've got proper coverage on the doc site, we should be good to remove usage files and possibly provide guidance on how to add/update doc examples that will get referenced in the doc site.

I'm uncertain about the advantages here. It seems like the documentation site offers more detailed examples that often encompass most usage scenarios. While the codebase contains numerous usage files with minor differences, such as adding a property or two to a component, the documentation site could likely achieve the same effect with a toggle.

Agreed. For context, we are removing usage files for a couple of reasons:

  1. we aim for the doc site to serve as definitive source of usage examples
  2. currently, the doc site does not use/reference usage files
  3. generated readmes offer incomplete documentation compared to what the doc site provides

@DitwanP @macandcheese LMK if I missed anything.

@DitwanP
Copy link
Contributor Author

DitwanP commented Apr 9, 2024

The list below Is what I've found for usage samples that might need to get recreated on the doc site, not sure if they all need to so @macandcheese feel free to LMK what we should exclude if any.

  • Button
    • Internals sample?
  • Input
    • Native Events?
  • Label
    • Browser Caveat?
  • Modal
    • Before close
  • Popover
    • Virtual
  • Sheet
    • Before close
  • Shell Panel
    • With custom element?
  • Tooltip
    • Virtual

@macandcheese
Copy link
Contributor

The list below Is what I've found for usage samples that might need to get recreated on the doc site, not sure if they all need to so @macandcheese feel free to LMK what we should exclude if any.

  • Button

We can remove this one - this is outdated. We have explicit properties and don't blindly spread anything anymore.

  • Input

Defer to others on this one but it's not specific to Input, this could probably be in the core concepts section. Events is pretty sparse now: https://developers.arcgis.com/calcite-design-system/core-concepts/#events

If we expect certain components will have questions about using native events, maybe we can link to the above? Maybe a note on bubbling in certain situations for nested / slotted components?

  • Label

Not sure if this applies to our supported browsers anymore. With no more IE support we might be able to drop this?

  • Modal

We should definitely add something about this to any component with a beforeClose option - probably in the Best Practices section. We can also add examples with new samples showing a real-world use case, like an "Are you sure" prompt when trying to close a Modal, etc.

  • Popover

Defer to @driskull

  • Sheet

Same as Modal

  • Shell Panel

Let's confirm this class is still needed / if so we can have a note in Recommendations about this.

  • Tooltip

Defer to @driskull

@driskull
Copy link
Member

driskull commented Apr 9, 2024

We should add the virtual element doc for popover and tooltip.

@jcfranco
Copy link
Member

jcfranco commented Apr 9, 2024

Thanks y'all!

I agree with @macandcheese and @driskull's recommendations.

Copy link
Contributor

This PR has been automatically marked as stale because it has not had recent activity. Please close your PR if it is no longer relevant. Thank you for your contributions.

@github-actions github-actions bot added the Stale Issues or pull requests that have not had recent activity. label Apr 17, 2024
@DitwanP DitwanP removed the Stale Issues or pull requests that have not had recent activity. label May 21, 2024
@DitwanP
Copy link
Contributor Author

DitwanP commented May 21, 2024

The usage snippets that were not previously represented on the doc site has now been added, everything else was already covered by the existing samples.

@DitwanP DitwanP marked this pull request as ready for review May 21, 2024 18:38
@benelan
Copy link
Member

benelan commented May 24, 2024

This is going the opposite direction of what I had hoped we would do 😭.

It would be great if we could have the usage examples source of truth in the monorepo right next to the source code and API reference.

When I discussed this with @macandcheese before it sounded like there were just some formatting issues/inconsistencies that prevented him from being able to parse the usage example strings and use them in the doc site.

I think we should be standardizing the format of the usage examples, and from there I could write a simple parser to grab the examples, their descriptions, etc. from docsjson for use in the doc site.

Speaking for myself, I'm way more likely to learn from and contribute to usage examples if they are in the monorepo, rather than hidden behind a VPN.

Additionally, people may be using the usage fields of docsjson for something (although we don't have it documented so removing it wouldn't be a breaking change)

https://cdn.jsdelivr.net/npm/@esri/calcite-components@2.8.3/dist/extras/docs-json.json

@jcfranco
Copy link
Member

@benelan Thanks for sharing your thoughts. I understand where you're coming from, but at the moment, there's no benefit in having these files in both this repo and the doc site.

IIRC, there were some technical challenges in consuming usage files, and for practical reasons, the doc site was chosen to host them. If we can figure out how to consume usage files from the monorepo on the doc site, I would agree that the monorepo would be the best place for these files as the source of truth. It would also be beneficial to have them reside close to the component source, tests, and demo pages.

Maybe we can start a separate thread with @macandcheese, @geospatialem, and @DitwanP to brainstorm ideas on how to get this working.

@macandcheese
Copy link
Contributor

macandcheese commented May 24, 2024

To me it also comes down to purpose - the doc site samples often contain a mix of components placed together to illustrate common patterns, component layout, etc., and contain js, css, etc., that may not be related to the "root component" being described.

We can access the usage files through the docs-json just fine, but the consuming component sample (which, lives in the doc site now, but may live somewhere else at a later date) relies on frontmatter / metadata in the source markdown that's not relevant or needed in the monorepo (unique id, pretty name, description, etc).

Additionally, in the consuming component sample - we rely on a configEnabled property added to the source code to know which components to allow users to configure via knobs, similar to how in Storybook you need to specify the knobs explicitly in the source code. I don't think we'd want that property to be shown in the monorepo - in the component sample we of course strip it out and don't display it.

Copy link
Contributor

github-actions bot commented Jun 2, 2024

This PR has been automatically marked as stale because it has not had recent activity. Please close your PR if it is no longer relevant. Thank you for your contributions.

@github-actions github-actions bot added the Stale Issues or pull requests that have not had recent activity. label Jun 2, 2024
@github-actions github-actions bot removed the Stale Issues or pull requests that have not had recent activity. label Jun 6, 2024
@benelan benelan changed the base branch from main to dev June 10, 2024 09:15
Copy link
Contributor

This PR has been automatically marked as stale because it has not had recent activity. Please close your PR if it is no longer relevant. Thank you for your contributions.

@github-actions github-actions bot added the Stale Issues or pull requests that have not had recent activity. label Jun 24, 2024
@DitwanP DitwanP removed the Stale Issues or pull requests that have not had recent activity. label Jul 11, 2024
@DitwanP DitwanP added the skip visual snapshots Pull requests that do not need visual regression testing. label Jul 11, 2024
@DitwanP DitwanP merged commit eb8bcc6 into dev Jul 11, 2024
11 checks passed
@DitwanP DitwanP deleted the dit13711/remove-component-usage-code branch July 11, 2024 18:46
@github-actions github-actions bot added this to the 2024-07-30 - Jul Release milestone Jul 11, 2024
benelan added a commit that referenced this pull request Jul 15, 2024
…-to-monorepo

* origin/dev: (206 commits)
  fix(tile): center align slot's text when alignment is equal to center (#9773)
  build: regenerate package lock due to build errors (#9774)
  build(deps): update dependency @floating-ui/dom to v1.6.7 (#9766)
  chore(common-tests): add themed test helper (#9763)
  chore: release next
  fix(shell): update shell to correctly position calcite shell panel at shell's bottom (#9748)
  chore: release next
  fix(tile): center align contentTop and contentBottom slots when alignment prop equals "center" (#9732)
  chore: release next
  chore(tree-item): fix mutable warning `indeterminate` prop (#9760)
  fix(panel, flow-item): fix footer-padding CSS prop regression (#9757)
  build(deps): update angular monorepo to v18 (major) (#9587)
  chore: release next
  Chore: remove component usage files (#9052)
  fix(input-number): restore decimal input mode default (#9741)
  test(panel, flow-item): add scale control to simple stories (#9747)
  chore: release next
  feat(panel, flow-item): add scale property (#9730)
  chore: release next
  fix(segmented-control): Make check state update correctly (#9733)
  ...
calcite-admin pushed a commit that referenced this pull request Jul 30, 2024
**Related Issue:** #9015 

## Summary
Removing usage sample files from repo since we now have links to each
components page on the doc site for more in depth information and
samples.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Issues with changes that don't modify src or test files. skip visual snapshots Pull requests that do not need visual regression testing.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants