-
Notifications
You must be signed in to change notification settings - Fork 0
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
wip: create the documentation for the module 'forms' in flottform #54
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: nidhal-labidi <[email protected]>
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 think we don't really need to comment private methods. Can we gitignore the result of tsdoc and do that on merge to main or something?
/** | ||
* Stops polling for ICE candidates. | ||
*/ | ||
private stopPollingForIceCandidates = async () => { |
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 think this is obvious. Since this is private, I don't think we need a comment for those
Signed-off-by: nidhal-labidi <[email protected]>
Signed-off-by: nidhal-labidi <[email protected]>
The HTML containing the documentation can be created by running the script |
Signed-off-by: nidhal-labidi <[email protected]>
Signed-off-by: nidhal-labidi <[email protected]>
Signed-off-by: nidhal-labidi <[email protected]>
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.
Nice that this works! We need to get this into the build for the website or a subdomain at least then. We may also want to look into using custom styles to be able to have a look similar to the flottform.io page
// @ts-ignore: Unused variable | ||
id, |
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 think we should remove this completely
// @ts-ignore: Unused variable | |
id, |
* | ||
* @param {Object} params - Configuration options for setting up the Flottform component. | ||
* @param {HTMLElement} params.flottformAnchorElement - The HTML element to which the Flottform component will be attached. It determines where the UI will be built on the page. | ||
* @param {string} [params.id] - Optional ID for the UI used for File of Text element. |
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 think this should be removed here. It's in the createFileInput
and createTextInput
already, so it's never used.
* @param {string} [params.id] - Optional ID for the UI used for File of Text element. |
// @ts-ignore: Unused variable | ||
this.openPeerConnection.oniceconnectionstatechange = (e) => { |
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.
Should we read the iceConnectionState
from e
instead of using this.openPeerConnection.iceConnectionState
?
// @ts-ignore: Unused variable | ||
const noop = () => {}; |
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.
If we're not using this anymore, I think we should remove it 😅
// @ts-ignore: Unused variable | ||
export const createChannelQrCodeCss = (styles?: Styles) => `max-wigth: 100%; |
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.
What is max-wigth
? Let's remove all the Unused variables
if we're not using them. If we should have documentation for them, that documentation should be added somewhere already...
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 whole file will be removed
// @ts-ignore: Unused variable | ||
private logger: Logger; |
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.
Should we add verbose log statements?
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 think we can delete this file since we don't actually use it anymore
Signed-off-by: nidhal-labidi <[email protected]>
Signed-off-by: nidhal-labidi <[email protected]>
Signed-off-by: nidhal-labidi <[email protected]>
In the last commit (0090a4d) I've added a new library (https://www.npmjs.com/package/typedoc-material-theme) in order to style the docs. If you don't think that's a good idea we can remove it and just use the file |
Signed-off-by: nidhal-labidi <[email protected]>
Checklist
Reviewer
Changes
Create a documentation for the module 'forms' in Flottform using TSDoc and Typedoc