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

[feat][Remote objects] Edit a connection (for pg) #5210

Merged
merged 17 commits into from
Apr 30, 2024

Conversation

ijreilly
Copy link
Collaborator

@ijreilly ijreilly commented Apr 29, 2024

Context

#4774

How was it tested

Locally

In further PRs

  • Update connection status upon page change
  • Adapt Info banner to dark mode
  • placeholders for form

@@ -9,8 +9,8 @@ export type InfoAccent = 'blue' | 'danger';
export type InfoProps = {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

In a later PR I will update this component for it to be better responsive + add color variation for dark mode

@@ -143,7 +144,7 @@ const TextInputV2Component = (
{label && <StyledLabel>{label + (required ? '*' : '')}</StyledLabel>}
<StyledInputContainer>
<StyledInput
autoComplete="off"
autoComplete={autoComplete || 'off'}
Copy link
Collaborator Author

@ijreilly ijreilly Apr 29, 2024

Choose a reason for hiding this comment

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

Seeking to disable autocomplete here as it does not make sense for the db and will overwrite the default values from data.
"off" was not working (maybe it's not doing what I think it tries to do - as I don't see why we would want to always disable autocomplete?).

https://developer.mozilla.org/en-US/docs/Web/Security/Securing_your_site/Turning_off_form_autocompletion

(...)
For this reason, many modern browsers do not support autocomplete="off" for login fields:
If a site sets autocomplete="off" for a form, and the form includes username and password input fields, then the browser still offers to remember this login, and if the user agrees, the browser will autofill those fields the next time the user visits the page.
If a site sets autocomplete="off" for username and password fields, then the browser still offers to remember this login, and if the user agrees, the browser will autofill those fields the next time the user visits the page.

@@ -14,7 +14,7 @@ const StyledWrapper = styled.nav`
font-size: ${({ theme }) => theme.font.size.lg};
font-weight: ${({ theme }) => theme.font.weight.semiBold};
gap: ${({ theme }) => theme.spacing(2)};
line-height: ${({ theme }) => theme.text.lineHeight.md};
line-height: ${({ theme }) => theme.text.lineHeight.lg};
Copy link
Collaborator Author

@ijreilly ijreilly Apr 29, 2024

Choose a reason for hiding this comment

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

Im aware this will change Breadcrumb lineheight everywhere but I think its a good thing as the Breadcrumb currently slightly glitches everywhere there is a save section on the same line:

glitch.mov

@ijreilly ijreilly changed the title 4774 edit server connection 2 [feat][Remote objects] Edit a connection (for pg) Apr 29, 2024
id: connectionId,
});

navigate(
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

TODO in a later PR - when redirecting to this page, connection status is not updated

Enregistrement.de.l.ecran.2024-04-29.a.16.01.50.mov

Copy link
Member

@charlesBochet charlesBochet left a comment

Choose a reason for hiding this comment

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

Frontend code is perfect!

@ijreilly ijreilly marked this pull request as ready for review April 30, 2024 15:46
@ijreilly ijreilly merged commit 1b2ed80 into main Apr 30, 2024
4 of 9 checks passed
@ijreilly ijreilly deleted the 4774-edit-server-connection-2 branch April 30, 2024 15:46
charlesBochet added a commit that referenced this pull request May 1, 2024
Following #5210

---------

Co-authored-by: Charles Bochet <[email protected]>
arnavsaxena17 pushed a commit to arnavsaxena17/twenty that referenced this pull request Oct 6, 2024
## Context
twentyhq#4774 

## How was it tested
Locally

## In further PRs
- Update connection status upon page change
- Adapt Info banner to dark mode
- placeholders for form
arnavsaxena17 pushed a commit to arnavsaxena17/twenty that referenced this pull request Oct 6, 2024
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