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.

Sign up for GitHub

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

Jump to bottom

Updated visible 'Twitter' name to 'X' in Personal info #41041

Merged
merged 2 commits into from
Nov 10, 2023
Merged

Updated visible 'Twitter' name to 'X' in Personal info #41041

merged 2 commits into from
Nov 10, 2023

Conversation

gcald
Copy link
Contributor

@gcald gcald commented Oct 22, 2023 •
edited
Loading

Summary

  • Updated the visual instances of 'Twitter' to 'X' in the setting's Personal Info tab

Before:

image

After:

image

TODO

  • Read through the checklist

Checklist

@gcald gcald force-pushed the update-twitter-to-x-in-personal-settings branch from a595ab1 to fda0c3c Compare October 22, 2023 01:39
szaimen
szaimen reviewed Oct 22, 2023
View reviewed changes
apps/settings/src/components/PersonalInfo/TwitterSection.vue Outdated
@@ -22,7 +22,7 @@

<template>
<AccountPropertySection v-bind.sync="twitter"
:placeholder="t('settings', 'Your Twitter handle')" />
:placeholder="t('settings', 'Your X handle')" />
Copy link
Contributor

@szaimen szaimen Oct 22, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe?

Suggested change
:placeholder="t('settings', 'Your X handle')" />
:placeholder="t('settings', 'Your X
(formerly Twitter) handle')" />

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion! I've updated the PR.

@szaimen szaimen added the 3. to review Waiting for reviews label Oct 22, 2023
@szaimen szaimen added this to the Nextcloud 28 milestone Oct 22, 2023
@szaimen szaimen requested review from a team, artonge, nfebe and sorbaugh and removed request for a team October 22, 2023 08:01
szaimen
szaimen reviewed Oct 22, 2023
View reviewed changes
apps/settings/src/constants/AccountPropertyConstants.js Outdated
@@ -58,7 +58,7 @@ export const ACCOUNT_PROPERTY_READABLE_ENUM = Object.freeze({
PHONE: t('settings', 'Phone number'),
PROFILE_ENABLED: t('settings', 'Profile'),
ROLE: t('settings', 'Role'),
TWITTER: t('settings', 'Twitter'),
TWITTER: t('settings', 'X (Formerly Twitter)'),
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
TWITTER: t('settings', 'X (Formerly Twitter)'),
TWITTER: t('settings', 'X (formerly Twitter)'),

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion! I've updated the PR.

@gcald gcald requested a review from szaimen October 22, 2023 21:55
@gcald gcald force-pushed the update-twitter-to-x-in-personal-settings branch 2 times, most recently from 9a7cc39 to f0f1e49 Compare October 28, 2023 22:38
@gcald
Copy link
Contributor Author

gcald commented Oct 28, 2023

Sorry, this is my first time contributing to this project and I'm a bit confused. What are the next steps for getting this PR merged in? Also, I have a few questions:

  • Do I need to build the project and commit the /dist/ folder?

  • I saw some checks were failing in a previous commit. How can I parse these to see what tests I need to update?

    • I believe the Cypress runners failed, but I can't get these to work locally.

  • I'm having an issue getting my local version of the tests to match the results of the workflow checks.

If anyone can help answer these questions or point me in the right direction, that would be greatly appreciated. Thanks!

@skjnldsv skjnldsv mentioned this pull request Nov 1, 2023
Merged
@blizzz blizzz mentioned this pull request Nov 6, 2023
Merged
@blizzz

This comment was marked as outdated.

Sign in to view
@blizzz blizzz mentioned this pull request Nov 10, 2023
Merged
gcald added 2 commits November 10, 2023 17:42
@gcald @AndyScherzinger
Updated visible 'Twitter' name to 'X' in Personal info
4871f59 
Signed-off-by: George Calderon <gcald117@gmail.com>
@gcald @AndyScherzinger
Updated visible 'Twitter' name to 'X' in Personal info
a047686 
Signed-off-by: George Calderon <gcald117@gmail.com>
@AndyScherzinger AndyScherzinger force-pushed the update-twitter-to-x-in-personal-settings branch from f0f1e49 to a047686 Compare November 10, 2023 16:43
@AndyScherzinger
Copy link
Member

/compile /

@AndyScherzinger AndyScherzinger mentioned this pull request Nov 10, 2023
Merged
5 tasks
@AndyScherzinger AndyScherzinger merged commit a047686 into nextcloud:master Nov 10, 2023
66 of 80 checks passed
@welcome Welcome
Copy link

welcome bot commented Nov 10, 2023

Thanks for your first pull request and welcome to the community! Feel free to keep them coming! If you are looking for issues to tackle then have a look at this selection: https://github.com/nextcloud/server/issues?q=is%3Aopen+is%3Aissue+label%3A%22good+first+issue%22

@AndyScherzinger
Copy link
Member

Merged via #41385

Thanks a lot for the PR @gcald

To answer your questions

  • Do I need to build the project and commit the /dist/ folder?

Yes the dist folder needs to be commited

  • I saw some checks were failing in a previous commit. How can I parse these to see what tests I need to update?
  • I believe the Cypress runners failed, but I can't get these to work locally.

Test failures were not related to your PR, you can view the output at the bottom of PRs on the "Details" link of the failing Github action.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nfebe nfebe nfebe approved these changes

@artonge artonge artonge approved these changes

@sorbaugh sorbaugh Awaiting requested review from sorbaugh sorbaugh was automatically assigned from nextcloud/server-frontend

@szaimen szaimen Awaiting requested review from szaimen

Assignees
No one assigned
Labels
3. to review Waiting for reviews
Projects
None yet
Milestone
Nextcloud 28
Development

Successfully merging this pull request may close these issues.
6 participants
@gcald @blizzz @AndyScherzinger @artonge @nfebe @szaimen