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

Add new input cookie_domain, fix styles for Custom component #15

Merged
merged 14 commits into from
Feb 1, 2023

Conversation

aringocode
Copy link
Collaborator

@aringocode aringocode commented Jan 24, 2023

Changes Client

  1. add new input to ClientCreateContainer cookie_domain
  2. add validation for cookie_domain input
  3. add row with cookie_domain in ClientDetailContainer
  4. add condition, if client_name and redirect_uris have not been edited, save button will be disabled
  5. add locales for new text
clietndetail.mp4

Changes Credentials Custom component

  1. Fixing styles, if screen height is reduced, elements in the Custom comopent do not move
credential-custom-component.mp4

@aringocode aringocode self-assigned this Jan 24, 2023
@aringocode aringocode requested a review from Pe5h4 January 24, 2023 13:43
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.

Missing changelog

"Redirect URIs": "URIs k přesměrování",
"Create client": "Vytvořit klienta",
"Unknown item": "Neznámá položka"
"Unknown item": "Neznámá položka",
"Required": "Povinné"
Copy link
Collaborator

Choose a reason for hiding this comment

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

@Aringoaway Should be Required field: Povinný údaj

<Button
color="primary"
type="submit"
title={isDirty ? t("ClientDetailContainer|Save changes") : t("ClientDetailContainer|No changes were made")}
Copy link
Collaborator

Choose a reason for hiding this comment

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

@Aringoaway Out of curiosity, why do you need isDirty here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I need it, that to see if there have been any changes in the inputs. If no changes have been made, the Save button will be disabled

@@ -187,9 +195,10 @@ const ClientCreateContainer = (props) => {
</FormGroup>
{metaData["properties"] && Object.entries(metaData["properties"]).map(([key, value]) => {
switch(key) {
case 'redirect_uris': return(<URiInput key={key} name={key} control={control} errors={errors} append={append} remove={remove} fields={fields} labelName={t("ClientCreateContainer|Redirect URIs")}/>)
case 'client_name': return(<TextInput key={key} name={key} register={register} labelName={t('ClientCreateContainer|Client name')}/>)
case 'redirect_uris': return(<URiInput key={key} name={key} control={control} errors={errors} append={append} remove={remove} fields={fields} title={t("ClientCreateContainer|Required")} labelName={t("ClientCreateContainer|Redirect URIs")}/>)
Copy link
Collaborator

Choose a reason for hiding this comment

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

@Aringoaway Why you need to add a title prop here? Cant it be managed within name prop?

@aringocode aringocode requested a review from Pe5h4 January 27, 2023 08:34
@aringocode aringocode changed the title Add new input cookie_domain Add new input cookie_domain, fix styles for Custom component Jan 27, 2023
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.

@Aringoaway Looks good, only minor inputs from me

@@ -187,9 +195,10 @@ const ClientCreateContainer = (props) => {
</FormGroup>
{metaData["properties"] && Object.entries(metaData["properties"]).map(([key, value]) => {
switch(key) {
case 'redirect_uris': return(<URiInput key={key} name={key} control={control} errors={errors} append={append} remove={remove} fields={fields} labelName={t("ClientCreateContainer|Redirect URIs")}/>)
case 'client_name': return(<TextInput key={key} name={key} register={register} labelName={t('ClientCreateContainer|Client name')}/>)
case 'redirect_uris': return(<URiInput key={key} name={key} control={control} errors={errors} append={append} remove={remove} fields={fields} labelName={t("ClientCreateContainer|Redirect URIs") + "*"}/>)
Copy link
Collaborator

Choose a reason for hiding this comment

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

@Aringoaway You can have labelName this way

labelName={`${t("ClientCreateContainer|Redirect URIs")}*`}

);
const isInvalid = (name) => {
if (name === "preferred_client_id" && (errors[name] != undefined)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

@Aringoaway Can be this condition simplified?

const isInvalid = (name) => {
		if (((name === "preferred_client_id") || (name === "cookie_domain")) && (errors[name] != undefined)) {
			return true;
		}
		return false;
}

@aringocode aringocode requested a review from Pe5h4 January 30, 2023 09:19
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.

@Aringoaway Only minor one I found just now, otherwise cool. Btw. update your branch from changes in main, I see that yours is out of date and there might be some conflicts

max-height: 15vh;
min-height: 15vh !important;
max-height: 200px;
min-height: 200px !important;
Copy link
Collaborator

Choose a reason for hiding this comment

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

@Aringoaway Cant be max-height and min-height replaced just with height ?

@aringocode aringocode requested a review from Pe5h4 January 31, 2023 07:06
@@ -9,8 +9,7 @@

//CredentialCustomDataContainer
Copy link
Collaborator

Choose a reason for hiding this comment

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

@Aringoaway Btw. I believe that SCSS styling for CustomDataContainer should be in separate customdata.scss file, since credentials.scss should consist only styles for credentials components, whereas CustomDataContainer is a container, which can be used also outside of credentials.

I know that it is some residual from the past, which was forgotten here, but I still think we should do it right. I hope you dont mind to refactor also this bit within this PR :) Thank you

@aringocode aringocode requested a review from Pe5h4 February 1, 2023 06:15
@aringocode aringocode marked this pull request as ready for review February 1, 2023 12:40
@aringocode aringocode merged commit 0339ad8 into main Feb 1, 2023
@aringocode aringocode deleted the refactor/client-add-new-input branch February 1, 2023 12:40
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