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

Refactor/customdata docs update #11

Merged
merged 13 commits into from
Jan 10, 2023
Merged

Conversation

petrKavulok
Copy link
Contributor

@petrKavulok petrKavulok commented Jan 4, 2023

This PR

  • Prevents passing wrong uri as props to the CustomData component by removing the first character if the first character happens to be a slash '/'
  • Documentation was added describing all props necessary and their types

Screenshot 2023-01-05 at 9 46 22

@petrKavulok petrKavulok self-assigned this Jan 4, 2023
@@ -73,8 +85,12 @@ export function CustomDataContainer({app, resources, customData, setCustomData,
const onSave = async (data) => {
// first, transform data to appropirate format for api request and skip pairs with empty key field
let objToSubmit = await prepForRequest(data);
// second, check if uri is in the right format to prevend invalid address
let address;
uri.charAt(0) === '/' ? address = uri.substring(1, uri.length)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@petrKavulok You dont need to have this construction of conditions here at all, you can use regex expression and do simply:

let address = url.replace(/^\//g, "");

This will always remove the first trailing slash if it will appear in the url string. If there will be no trailing slash, the url string will stay intact.

You can also make it even simplier by using this in the request (then you dont need to declare address variable)

let response = await SeaCatAuthAPI.put(`/${url.replace(/^\//g, "")}`, {"data": objToSubmit});

@@ -11,6 +11,18 @@ import {
FormGroup
} from 'reactstrap';

/*
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@petrKavulok Shouldnt this be in some doc file?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wasn't sure where to put it, since we have docs in asab-webui/doc but this is a part of seacat-admin-webui. I wasn't sure it was worth creating a /doc folder for a documentation of this magnitude. If it should be in the doc folder, should the folder be a part of the /components folder? or have a different spot in the treemenu?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@petrKavulok Nope, it should be in doc folder on top level (there already is a doc folder)

- asab-webui
- conf
- doc <<<< here is the doc folder where you want to add any broader description regarding of any component
- public
- src
- ....

btw. docs for asab-webui surely are in asab-webui/doc, but this is not the case, innit?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you're right, I missed that, I'm sorry :-)

@petrKavulok petrKavulok requested a review from Pe5h4 January 6, 2023 08:22
@@ -11,6 +11,18 @@ import {
FormGroup
} from 'reactstrap';

/*
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@petrKavulok Nope, it should be in doc folder on top level (there already is a doc folder)

- asab-webui
- conf
- doc <<<< here is the doc folder where you want to add any broader description regarding of any component
- public
- src
- ....

btw. docs for asab-webui surely are in asab-webui/doc, but this is not the case, innit?

@petrKavulok petrKavulok requested a review from Pe5h4 January 6, 2023 09:49
Copy link
Collaborator

@Pe5h4 Pe5h4 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small remarks. Oh and put the md file to some dedicated components dir within the docs


## Setup

Before using this component in your project, `react-i18next`, `reactstrap` and `react-hook-form` must be installed and added into the project's `package.json` file:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@petrKavulok I think here it is not necessary to mention it, since it can be used only within the seacat-admin-ui or where seacat-admin-ui is used as a submodule, but in all cases, you will need to have those libraries installed by default (it is already in package.json). Other case would be if it is a reusable component of asab-webui, but it is not.

yarn add react-hook-form
```

At the moment works ONLY with `apiPath='seacat_auth'`
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@petrKavulok Again, not necessary to mention that, since it is used only within seacat-admin ui and for that you need to have seacat_auth proxy set in general

...

<CustomDataaContainer
resources={resources}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@petrKavulok Watch out for padding here, it is not necessary to have it that huge


...

return (
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@petrKavulok Please format the example a bit better, that it is readable

@petrKavulok petrKavulok requested a review from Pe5h4 January 9, 2023 17:30
@petrKavulok petrKavulok marked this pull request as ready for review January 10, 2023 21:05
@petrKavulok petrKavulok merged commit d390a8c into main Jan 10, 2023
@petrKavulok petrKavulok deleted the refactor/customdata-docs-update branch January 10, 2023 21:06
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.

3 participants