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

Documented: dxpImage, dxpUserProfile ,dxpOmsInstanceNaviagtor components to understand their working #257

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

Ritika-Patel08
Copy link

Documented the dxp-components to understand their working such that they can be seamlessly integrated into projects according to the requirement.

Copy link
Contributor

@dt2patel dt2patel left a comment

Choose a reason for hiding this comment

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

Added some comments. Please take a look

<table>
<thead>
<tr>
<th>Prop Name</th>
Copy link
Contributor

Choose a reason for hiding this comment

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

we can just say "prop" here instead of "prop name"

<tbody>
<tr>
<td><code>src</code></td>
<td>It's specifying that the component expects a prop named src.n this component props.src to access the value of the src.</td>
Copy link
Contributor

Choose a reason for hiding this comment

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

confusingly worded, please clarify



## Recommendation
<li>Use the component to load and display images stored as local assets within the project directory.</li>
Copy link
Contributor

Choose a reason for hiding this comment

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

This information is redundant and doesn't add anything new from what was already written above. We should convey to the user:

What does this component offer as a value that users did not have already.
Some example use cases with code of how to use it in that scenario.



### state manamement
#### 1.useAuthStore()
Copy link
Contributor

Choose a reason for hiding this comment

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

this part feels incomplete in regards to what we are trying to communicate. How is this connected to this component. Is there a step 2?

### Events
#### 1. goToOms()
<li>The purpose of goToOms is to navigate the user to a different screen or page within the application that corresponds to a specific OMS instance.
<li>It takes two parameters: <b>token</b> (an authentication token) and <b>omsName</b> (the name of the OMS instance).
Copy link
Contributor

Choose a reason for hiding this comment

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

The example we have given above does not include these parameters. Are these given as props? are these values expected in a state somewhere?


## DxpUserProfile
### Introduction
<li>The User Profile Card component is designed to display user profile information, allowing users to see details such as their login ID and party name. It also provides options to perform actions like logging out and navigating to the launchpad.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
<li>The User Profile Card component is designed to display user profile information, allowing users to see details such as their login ID and party name. It also provides options to perform actions like logging out and navigating to the launchpad.
<li>The User Profile Card component is designed to display user profile information, allowing users to see details such as their login ID and party name. It also provides options to perform actions like logging out and navigating to the Launchpad.


### Events
#### 1. logout()
The logout() method is triggered when the user clicks on the logout button (ion-button). It initiates the logout process for the user.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
The logout() method is triggered when the user clicks on the logout button (ion-button). It initiates the logout process for the user.
The logout() method is triggered when the user clicks on the logout button, initiating the logout process for the user.

#### 2. goToLaunchpad()
The goToLaunchpad() method is invoked when the user clicks on the "Go To Launchpad" button. It redirects the user to the launchpad by changing the window location.

### CSS
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think we need to really go this deep into the code in this draft. Lets focus on the functionality and usage first.

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.

2 participants