-
Notifications
You must be signed in to change notification settings - Fork 1
Modernize docs #79
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
base: dev
Are you sure you want to change the base?
Modernize docs #79
Conversation
Reviewer's GuideModernizes auro-backtotop documentation, examples, and package configuration to align with current Auro docs layout, custom registration guidance, and auro-cli tooling, while tightening API typing and metadata. Class diagram for updated AuroBackToTop component APIclassDiagram
class LitElement
class AuroBackToTop {
<<customElement>>
+boolean disabled
+boolean hidden
+boolean interactionActive
+boolean lastScrollDirectionUp
+"primary | secondary" variant
+constructor()
+_initializeDefaults()
+handleScroll(event)
+handleClick(event)
+static register(name)
}
LitElement <|-- AuroBackToTop
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
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.
Hey - I've found 3 issues, and left some high level feedback:
- The updated description in
docs/partials/description.mdno longer explains that slotted text is shown when scrolling up; consider restoring that behavior detail so consumers understand when text vs icon appear. - Now that
hidden,interactionActive, andlastScrollDirectionUpno longer reflect to attributes, consider marking them as internal (e.g., via@privateJSDoc orattribute: false) so they don’t get treated as part of the public API surface.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The updated description in `docs/partials/description.md` no longer explains that slotted text is shown when scrolling up; consider restoring that behavior detail so consumers understand when text vs icon appear.
- Now that `hidden`, `interactionActive`, and `lastScrollDirectionUp` no longer reflect to attributes, consider marking them as internal (e.g., via `@private` JSDoc or `attribute: false`) so they don’t get treated as part of the public API surface.
## Individual Comments
### Comment 1
<location> `src/auro-backtotop.js:106-112` </location>
<code_context>
type: Boolean,
reflect: true,
},
hidden: {
- type: Boolean,
- reflect: true,
+ type: Boolean
},
interactionActive: {
- type: Boolean,
+ type: Boolean
},
lastScrollDirectionUp: {
- type: Boolean,
+ type: Boolean
</code_context>
<issue_to_address>
**issue (bug_risk):** Removing `reflect: true` from these internal state properties may break attribute-based styling or external control relying on attributes.
`hidden`, `interactionActive`, and `lastScrollDirectionUp` no longer reflect to attributes, so any CSS, integration code, or tooling that previously relied on those attributes will stop working without warning. If these flags are meant to remain observable/stylable from the outside, keep `reflect: true` on the relevant properties; otherwise, this change should be clearly treated as an intentional breaking change.
</issue_to_address>
### Comment 2
<location> `package.json:71-74` </location>
<code_context>
"./custom-elements.json": "./custom-elements.json",
"./readme.md": "./README.md",
- ".": "./dist/registered.js",
+ ".": {
+ "module": "./dist/registered.js",
+ "types": "./dist/index.d.ts",
+ "default": "./dist/registered.js"
+ },
"./demo/*.md": "./demo/*.md",
</code_context>
<issue_to_address>
**question (bug_risk):** Changing the root export from a string path to an object with `module`/`default` may impact consumers expecting the previous export shape.
This object form is more descriptive but may change how bundlers and CommonJS consumers resolve the entry, especially around the `default` key. Consider using standard condition keys like `import`/`require` and a top-level `types` field, or confirm that Node, bundlers, and our design system tooling all correctly handle this new export shape before merging.
</issue_to_address>
### Comment 3
<location> `docs/api.md:3` </location>
<code_context>
# auro-backtotop
-The auro-backtotop element provides users a way to quickly return to page top.
+The `auro-backtotop` element provides users a way to quickly return to page top.
### Properties & Attributes
</code_context>
<issue_to_address>
**suggestion (typo):** Consider adjusting the phrasing to "return to the top of the page" for more natural grammar.
You could also expand the sentence to: "The `auro-backtotop` element provides users with a way to quickly return to the top of the page," which adds the missing article and reads more naturally.
```suggestion
The `auro-backtotop` element provides users with a way to quickly return to the top of the page.
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
🚀 PR Release Published! To install: npm install @aurodesignsystem-dev/[email protected]Install via alias: npm install @aurodesignsystem/auro-backtotop@npm:@aurodesignsystem-dev/[email protected] |
Component Demo Linksbacktotop: https://auro-backtotop-79.surge.sh |
66a3907 to
070d299
Compare
example code updated to reflect latest Auro element APIs example documentation layout improved custom registration updated and moved to install page core element descriptions updated doc build system now uses the latest version of auro-cli package json configuration updated to support latest auro-cli
4238547 to
f7a8005
Compare
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.
Hey - I've found 3 issues, and left some high level feedback:
- In
AuroBackToTop.properties,hidden,interactionActive, andlastScrollDirectionUpare no longerreflect: true; if these were previously observable as attributes, consider whether this API change is intentional or if they should remain reflected for external styling/inspection. - The
disabledproperty description (“Render the trigger inline, will always be visible.”) is a bit confusing for a disabled state; consider refining this text in both the JSDoc and API table to more accurately describe whatdisabledactually does in the component. - The use-cases partial now reads “The
<auro-backtotop>element should be used in situations where users may:” followed by a single bullet; consider rephrasing or adjusting the list so the sentence and bullet formatting read more naturally.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `AuroBackToTop.properties`, `hidden`, `interactionActive`, and `lastScrollDirectionUp` are no longer `reflect: true`; if these were previously observable as attributes, consider whether this API change is intentional or if they should remain reflected for external styling/inspection.
- The `disabled` property description (“Render the trigger inline, will always be visible.”) is a bit confusing for a disabled state; consider refining this text in both the JSDoc and API table to more accurately describe what `disabled` actually does in the component.
- The use-cases partial now reads “The `<auro-backtotop>` element should be used in situations where users may:” followed by a single bullet; consider rephrasing or adjusting the list so the sentence and bullet formatting read more naturally.
## Individual Comments
### Comment 1
<location> `README.md:37-39` </location>
<code_context>
-For the most up to date information on [UI development browser support](https://auro.alaskaair.com/support/browsersSupport)
+<!-- AURO-GENERATED-CONTENT:START (FILE:src=./docs/partials/useCases.md) -->
+<!-- The below content is automatically added from ./docs/partials/useCases.md -->
+The `<auro-backtotop>` element should be used in situations where users may:
+* Pages with large amounts of content where a shortcut to jump to the top of the page is appropriate.
<!-- AURO-GENERATED-CONTENT:END -->
</code_context>
<issue_to_address>
**suggestion (typo):** Clarify the lead-in sentence and bullet so they form a grammatically complete construction.
The current lead-in + bullet combination (“The `<auro-backtotop>` element should be used in situations where users may:” + “Pages with large amounts of content…”) is grammatically awkward. Consider either:
- “The `<auro-backtotop>` element should be used in situations such as:”
- “Pages with large amounts of content where a shortcut to jump to the top of the page is appropriate.”
or rephrasing the bullet to start with “On pages with large amounts of content…” so the full construction reads smoothly.
```suggestion
The `<auro-backtotop>` element should be used in situations such as:
* Pages with large amounts of content where a shortcut to jump to the top of the page is appropriate.
```
</issue_to_address>
### Comment 2
<location> `docs/api.md:9` </location>
<code_context>
-| variant | variant | string | "primary" | The variant attribute allows for rendering the button using the primary (default) or secondary styles. |
+| Properties | Attributes | Modifiers | Type | Default | Description |
+| ---------- | ---------- | --------- | ------------------------ | --------- | ------------------------------------------------------------------------------------------------------ |
+| disabled | disabled | | boolean | `false` | Render the trigger inline, will always be visible. |
+| variant | variant | | `primary` \| `secondary` | `primary` | The variant attribute allows for rendering the button using the primary (default) or secondary styles. |
</code_context>
<issue_to_address>
**suggestion (typo):** Improve the grammar of the `disabled` property description.
The sentence “Render the trigger inline, will always be visible.” is ungrammatical. Please rephrase to something like “Renders the trigger inline so it is always visible.” for clearer documentation.
```suggestion
| disabled | disabled | | boolean | `false` | Renders the trigger inline so it is always visible. |
```
</issue_to_address>
### Comment 3
<location> `docs/partials/api.md:59` </location>
<code_context>
-</auro-accordion>
-
-#### ariaLabel
+### Aria Label
The `ariaLabel` slot allows you to pass an aria-label to the HTML5 button. The default value is `"arrow-up"`.
</code_context>
<issue_to_address>
**nitpick (typo):** Use standard capitalization for the ARIA acronym in the heading.
Consider updating the heading to “### ARIA Label” to match standard accessibility terminology. The rest of the description can stay the same.
```suggestion
### ARIA Label
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| The `<auro-backtotop>` element should be used in situations where users may: | ||
|
|
||
| * Pages with large amounts of content where a shortcut to jump to the top of the page is appropriate. |
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.
suggestion (typo): Clarify the lead-in sentence and bullet so they form a grammatically complete construction.
The current lead-in + bullet combination (“The <auro-backtotop> element should be used in situations where users may:” + “Pages with large amounts of content…”) is grammatically awkward. Consider either:
- “The
<auro-backtotop>element should be used in situations such as:”- “Pages with large amounts of content where a shortcut to jump to the top of the page is appropriate.”
or rephrasing the bullet to start with “On pages with large amounts of content…” so the full construction reads smoothly.
| The `<auro-backtotop>` element should be used in situations where users may: | |
| * Pages with large amounts of content where a shortcut to jump to the top of the page is appropriate. | |
| The `<auro-backtotop>` element should be used in situations such as: | |
| * Pages with large amounts of content where a shortcut to jump to the top of the page is appropriate. |
example code updated to reflect latest Auro element APIs example documentation layout improved
custom registration updated and moved to install page core element descriptions updated
doc build system now uses the latest version of auro-cli package json configuration updated to support latest auro-cli
Alaska Airlines Pull Request
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Reviewer Checklist:
Documentation Accuracy
Stringproperties displaying all of their "options"?indexpage (i.e. only the most "basic" example(s) of component)installpage and renders as expectedCode Changes
myExample.html->my-example.html)perfcommit to trigger a release, as expected?Visual Checks
See Codeaccordions actually render the code blocks under their respective examplesRC Checklist
Testing Checklist:
Browsers
Browsers Support Guide
Dev demo link
Android
iOS
Desktop
Scenarios
Framework playground
**By submitting this Pull Request, I confirm that my contribution is made under the terms of the Apache 2.0 license.**
Pull Requests will be evaluated by their quality of update and whether it is consistent with the goals and values of this project. Any submission is to be considered a conversation between the submitter and the maintainers of this project and may require changes to your submission.
Thank you for your submission!
-- Auro Design System Team
Summary by Sourcery
Modernize the auro-backtotop component documentation and package configuration to align with the latest Auro standards and APIs.
New Features:
Enhancements:
Build:
Documentation: