Closed PeterMPhillips closed 4 years ago
@PeterMPhillips please remember to move to "Review" pipeline, it is quite weird to see an "Approved" PR that is "In progress" pipeline.
Couple minor nit-picks:
- The text should be center-aligned in the Add another and Submit buttons
- There seems to be excessive space below the Add another button and above the Submit button
I'm not sure if these were previously resolved; I'm going off the animation in the original PR.
@stellarmagnet As far as I can tell the spacing is the same as the New Allocations panel. I think the '+' makes the 'Add another' button look off-centre. This may just be on my machine though. I can update the spacing for the 'Submit' button but will need to know what space you'd like. Right now its 24px.
Here is an image of New Allocation:
And here is an image of New Dot Vote:
Couple minor nit-picks:
- The text should be center-aligned in the Add another and Submit buttons
- There seems to be excessive space below the Add another button and above the Submit button
I'm not sure if these were previously resolved; I'm going off the animation in the original PR.
@stellarmagnet As far as I can tell the spacing is the same as the New Allocations panel. I think the '+' makes the 'Add another' button look off-centre. This may just be on my machine though. I can update the spacing for the 'Submit' button but will need to know what space you'd like. Right now its 24px.
Here is an image of New Allocation:
And here is an image of New Dot Vote:
For consistency, can we just use the same design that the client has? for example, in the onboarding panel, it shouldn't be too much work to adapt, and personally it looks much better with the icon than the raw + sign:
Is just this easy:
<Button
icon={
<IconPlus
css={`
color: ${theme.accent};
`}
/>
}
label="Add more"
onClick={addMember}
/>
Couple minor nit-picks:
- The text should be center-aligned in the Add another and Submit buttons
- There seems to be excessive space below the Add another button and above the Submit button
I'm not sure if these were previously resolved; I'm going off the animation in the original PR.
@stellarmagnet As far as I can tell the spacing is the same as the New Allocations panel. I think the '+' makes the 'Add another' button look off-centre. This may just be on my machine though. I can update the spacing for the 'Submit' button but will need to know what space you'd like. Right now its 24px. Here is an image of New Allocation: And here is an image of New Dot Vote:
For consistency, can we just use the same design that the client has? for example, in the onboarding panel, it shouldn't be too much work to adapt, and personally it looks much better with the icon than the raw + sign:
Is just this easy:
<Button icon={ <IconPlus css={` color: ${theme.accent}; `} /> } label="Add more" onClick={addMember} />
I've changed both the OptionsInput (used by Dot Voting) and RecipientsInput (used by Allocations) to the button style described by @ottodevs:
@stellarmagnet @javieralaves how does this look to you?
While I appreciate Otto’s suggestion, I think that is out of scope for this ticket. If we are to make this change, we should make it on all of our UIs (new allocation, new dot vote, new issue curation)
On Mon, Jan 6, 2020 at 21:31 Peter Phillips notifications@github.com wrote:
Couple minor nit-picks:
- The text should be center-aligned in the Add another and Submit buttons
- There seems to be excessive space below the Add another button and above the Submit button
I'm not sure if these were previously resolved; I'm going off the animation in the original PR.
@stellarmagnet https://github.com/stellarmagnet As far as I can tell the spacing is the same as the New Allocations panel. I think the '+' makes the 'Add another' button look off-centre. This may just be on my machine though. I can update the spacing for the 'Submit' button but will need to know what space you'd like. Right now its 24px. Here is an image of New Allocation: [image: Screenshot from 2020-01-02 09-56-59] https://user-images.githubusercontent.com/19808076/71683051-495cca80-2d46-11ea-942a-855b0e721ae9.png And here is an image of New Dot Vote: [image: Screenshot from 2020-01-02 09-48-50] https://user-images.githubusercontent.com/19808076/71682920-f4b94f80-2d45-11ea-8404-7e6de0445648.png
For consistency, can we just use the same design that the client has? for example, in the onboarding panel, it shouldn't be too much work to adapt, and personally it looks much better with the icon than the raw + sign:
[image: image] https://user-images.githubusercontent.com/5030059/71706676-ac9e3980-2de6-11ea-8099-bea5f6d33129.png
Is just this easy:
<Button icon={ <IconPlus css={` color: ${theme.accent}; `} /> } label="Add more" onClick={addMember} />
I've changed both the OptionsInput (used by Dot Voting) and RecipientsInput (used by Allocations) to the button style described by @ottodevs https://github.com/ottodevs: [image: Screenshot from 2020-01-06 12-25-19] https://user-images.githubusercontent.com/19808076/71846836-59471800-3080-11ea-9012-46711c13ef37.png
@stellarmagnet https://github.com/stellarmagnet @javieralaves https://github.com/javieralaves how does this look to you?
— You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub https://github.com/AutarkLabs/open-enterprise/pull/1801?email_source=notifications&email_token=AATW7LIU5AL3PVVXTFTPZ53Q4OIJXA5CNFSM4J4PLWXKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIGWAWI#issuecomment-571301977, or unsubscribe https://github.com/notifications/unsubscribe-auth/AATW7LP2ENT3GMO2K2LN2CTQ4OIJXANCNFSM4J4PLWXA .
@stellarmagnet Aside from reverting back the changes. Is there anything else you want me to adjust on the Add another
button?
@PeterMPhillips - there are no other changes I see, I just think it's about maintaining consistency! But if it's easier, I'm not opposed to shipping as is, if it expedites the process, and we can create backlog tickets to update the other panels to your currently implemented design in C1.
The PR adds informative dot voting to the Dot Voting app. It covers everything in this epic: https://github.com/AutarkLabs/product/issues/48
Just a note, in the gif it shows the error message: 'Options may not be identical' but it has been updated to 'The same option cannot be specified twice.'