Change: confirm button text to create motion #3528

Julianahlk · 2022-06-22T16:25:53Z

Description

This PR is for changing the Confirm button text to Create Motion when creating a motion.

New stuff ✨

Updated text for the confirm button for all forms that create motions. Example:

Changes 🏗

The button text will need to change back to Confirm if the action is forced.
If the extension Governance ( Reputation Weight ) is disabled the button should show the text confirm.

Example:

(Resolves | Contributes to) #3503

CLAassistant · 2022-06-22T16:25:58Z

All committers have signed the CLA.

arrenv · 2022-06-27T07:26:58Z

@Julianahlk To request a review from everyone, you can use the JoinColony/frontend team option.

.=

arrenv

Works great, nice one @Julianahlk!! Congrats on your first PR 🥇

Julianahlk · 2022-06-27T08:06:06Z

Thanks @arrenv I will use that tag next time. Happy it works as intended. Thanks for reviewing it!

chinins

Great work for your first PR. There are 2 things to correct:

I think that you forgot the NetworkContractUpgradeDialog
In the create motion version of the button the text is not centred as per below. It's a tricky thing to notice and I paid attention because I had to update the button styles in another PR to fix this.

willm30

Nice PR! I presume the dialogs that have not been changed (e.g. ManageWhitelistDialogForm (Manage Address Book)) haven't been changed because they don't require a motion to be submitted? Or is every dialog supposed to have this "Create Motion" text?

Julianahlk · 2022-06-27T16:38:09Z

@chinins Oh I nice, thanks for catching that one I missed.
I will center the text. That's something I didn't check because I thought the button would re-size automatically. Thanks for reviewing!

rdig · 2022-06-27T21:08:47Z

Nice PR! I presume the dialogs that have not been changed (e.g. ManageWhitelistDialogForm (Manage Address Book)) haven't been changed because they don't require a motion to be submitted? Or is every dialog supposed to have this "Create Motion" text?

You are correct, the "missing" dialogs do not create a Motion, as they are basically cheating behind the scene and do not call a contract function, in the same way as the others do

rdig · 2022-06-27T21:10:00Z

@Julianahlk

I will center the text. That's something I didn't check because I thought the button would re-size automatically. Thanks for reviewing!

For this you'll have to add local styles that overwrite the Button component's default styles. Have a look at other examples and you'll catch on

rdig

Other than what Olga found, I'm good with this.

I'm pre-approving, but do please address Olga's concerns before merging

Julianahlk · 2022-06-28T14:32:21Z

For this you'll have to add local styles that overwrite the Button component's default styles. Have a look at other examples and you'll catch on.

On it @rdig ! Thank you for the brief explanation.

Julianahlk · 2022-06-28T21:50:26Z

@chinins @rdig

I switched the width property to be minWidth so the Create Motion/Confirm button could expand to fit any future size text (localized text). I am not sure if this what you were looking for but I think it is more future proof as the buttons can handle longer text if necessary. Let me know what you think.

rdig

Good, but I'd prefer your change be done inside CSS, rather then JS (components)

Also, you have a bad merge in your branch

If you were attempting to update the branch, you did it the wrong way around, as we use git rebase for that.

Let me know if you need help restoring your branch

rdig · 2022-06-28T22:22:25Z

...nents/Dialogs/AwardAndSmiteDialogs/ManageReputationDialogForm/ManageReputationDialogForm.tsx

          loading={isSubmitting}
          disabled={!isValid || inputDisabled}
-          style={{ width: styles.wideButton }}
+          style={{ minWidth: styles.wideButton }}


We usually prefer to add this via css styles not via JS.

You can do the same, by overwriting the button min-width in the styles file

@rdig So my goal was to remove the width style and replace it with minWidth. Are you saying that having the width style in the tsx file wasnt the best way to do it to begin with?

Exactly. But now that I'm thinking more about this, in order to expedite this PR, leave it like it is for now.

Let's just fix your branch tommorow, then you can merge it

rdig · 2022-06-28T22:30:09Z

src/modules/core/components/InfoPopover/MemberInfoPopover/UserReputation.tsx

+      {isUserReputationLoading ? (
+        <SpinnerLoader
+          appearance={{
+            theme: 'grey',
+            size: 'small',
+          }}
+        />


How did this changes end up in here ? These are not related to your branch.

Did you cherry-pick them from somewhere? If so, why ?

It looks like there are these commits from @arrenv earlier today that ended up in my branch, 0f5a455
I think this is the cause of these changes.

Yeah, some rebase / merge screwed up your branch. We'll fix it tommorow.

Julianahlk · 2022-06-28T23:19:56Z

@rdig Regarding the bad merge. Im not sure why this happened this time because I did try to use, git pull --rebase origin master. It seems like it was a merge of my own branch into my branch. I could use help restoring this.

More or less fine as it is.

Julianahlk · 2022-06-30T12:46:28Z

@chinins I implemented the changes you suggested and solved my merge conflicts with Raul yesterday. Can you please take a look at this PR again whenever you have a chance? Thank you!

chinins

All good - nice work!

Julianahlk · 2022-06-30T20:21:55Z

All good - nice work!

Thank you!

… button styles

Julianahlk added the feature label Jun 22, 2022

Julianahlk self-assigned this Jun 22, 2022

Julianahlk marked this pull request as ready for review June 24, 2022 15:38

Julianahlk requested review from danbr, rdig, ArmandoGraterol, chinins, willm30 and arrenv June 24, 2022 15:38

arrenv approved these changes Jun 27, 2022

View reviewed changes

chinins suggested changes Jun 27, 2022

View reviewed changes

willm30 reviewed Jun 27, 2022

View reviewed changes

rdig approved these changes Jun 27, 2022

View reviewed changes

rdig previously requested changes Jun 28, 2022

View reviewed changes

rdig reviewed Jun 28, 2022

View reviewed changes

Julianahlk force-pushed the feature/3503-create-motion-button branch from 7363973 to 6b07a18 Compare June 29, 2022 11:07

chinins approved these changes Jun 30, 2022

View reviewed changes

Change: confirm button text to create motion

59fb769

Julianahlk added 3 commits June 30, 2022 22:59

feat: change button text on force toggle state

7b82f4a

feat: added conditional logic for button text

a754f80

Feat: Added changes to NetworkContractUpgradeDialog and updated the…

0baa98a

… button styles

Julianahlk force-pushed the feature/3503-create-motion-button branch from d9d01ce to f3ec627 Compare June 30, 2022 21:08

rdig force-pushed the feature/3503-create-motion-button branch from f3ec627 to 0baa98a Compare July 1, 2022 08:25

Julianahlk merged commit 4a75bee into master Jul 1, 2022

Julianahlk deleted the feature/3503-create-motion-button branch July 1, 2022 09:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Change: confirm button text to create motion #3528

Change: confirm button text to create motion #3528

Julianahlk commented Jun 22, 2022 •

edited

Loading

CLAassistant commented Jun 22, 2022 •

edited

Loading

arrenv commented Jun 27, 2022 •

edited

Loading

arrenv left a comment

Julianahlk commented Jun 27, 2022

chinins left a comment •

edited

Loading

willm30 left a comment

Julianahlk commented Jun 27, 2022 •

edited

Loading

rdig commented Jun 27, 2022

rdig commented Jun 27, 2022

rdig left a comment

Julianahlk commented Jun 28, 2022 •

edited

Loading

Julianahlk commented Jun 28, 2022 •

edited

Loading

rdig left a comment

rdig Jun 28, 2022

Julianahlk Jun 28, 2022

rdig Jun 28, 2022

rdig Jun 28, 2022

Julianahlk Jun 28, 2022

rdig Jun 28, 2022

Julianahlk commented Jun 28, 2022 •

edited

Loading

Julianahlk commented Jun 30, 2022

chinins left a comment

Julianahlk commented Jun 30, 2022

Change: confirm button text to create motion #3528

Change: confirm button text to create motion #3528

Conversation

Julianahlk commented Jun 22, 2022 • edited Loading

Description

CLAassistant commented Jun 22, 2022 • edited Loading

arrenv commented Jun 27, 2022 • edited Loading

arrenv left a comment

Choose a reason for hiding this comment

Julianahlk commented Jun 27, 2022

chinins left a comment • edited Loading

Choose a reason for hiding this comment

willm30 left a comment

Choose a reason for hiding this comment

Julianahlk commented Jun 27, 2022 • edited Loading

rdig commented Jun 27, 2022

rdig commented Jun 27, 2022

rdig left a comment

Choose a reason for hiding this comment

Julianahlk commented Jun 28, 2022 • edited Loading

Julianahlk commented Jun 28, 2022 • edited Loading

rdig left a comment

Choose a reason for hiding this comment

rdig Jun 28, 2022

Choose a reason for hiding this comment

Julianahlk Jun 28, 2022

Choose a reason for hiding this comment

rdig Jun 28, 2022

Choose a reason for hiding this comment

rdig Jun 28, 2022

Choose a reason for hiding this comment

Julianahlk Jun 28, 2022

Choose a reason for hiding this comment

rdig Jun 28, 2022

Choose a reason for hiding this comment

Julianahlk commented Jun 28, 2022 • edited Loading

Julianahlk commented Jun 30, 2022

chinins left a comment

Choose a reason for hiding this comment

Julianahlk commented Jun 30, 2022

Julianahlk commented Jun 22, 2022 •

edited

Loading

CLAassistant commented Jun 22, 2022 •

edited

Loading

arrenv commented Jun 27, 2022 •

edited

Loading

chinins left a comment •

edited

Loading

Julianahlk commented Jun 27, 2022 •

edited

Loading

Julianahlk commented Jun 28, 2022 •

edited

Loading

Julianahlk commented Jun 28, 2022 •

edited

Loading

Julianahlk commented Jun 28, 2022 •

edited

Loading