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

🚲 Cycle candidates #1584

Closed
wants to merge 7 commits into from
Closed

🚲 Cycle candidates #1584

wants to merge 7 commits into from

Conversation

p-sad
Copy link
Contributor

@p-sad p-sad commented Oct 4, 2021

See #1388
Potentially closes this issue; I'm not sure though, because I don't really understand what "preview mode" is.

@p-sad p-sad requested a review from jodator October 4, 2021 12:31
@jodator
Copy link
Contributor

jodator commented Oct 4, 2021

Potentially closes this issue; I'm not sure though, because I don't really understand what "preview mode" is.

The preview mode means that this modal should be possible to display when the user announces candidacy: #1406. But AFAICS should be doable and I think that this PR might close #1388.

Copy link
Contributor

@jodator jodator left a comment

Choose a reason for hiding this comment

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

Works nice :) But I think that we could add tests for cycling.

ps.: Please add an issue for handling the "Candidacy discussion thread" button.

<CopyButtonTemplate
square
size="small"
textToCopy={`${window.location.host}/#/members/${candidate?.member.id}`}
Copy link
Contributor

Choose a reason for hiding this comment

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

Wrong link to candidacy. I don't think we have a separate issue (but you might search for one).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I haven't found any issue that relates to this, and I think it might deserve its own issue. I think link to member preview had one.

Copy link
Contributor

Choose a reason for hiding this comment

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

So let's create a separate issue for handling links to candidates.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added #1590

@p-sad p-sad requested a review from jodator October 5, 2021 11:04
Copy link
Contributor

@jodator jodator left a comment

Choose a reason for hiding this comment

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

LGTM, but some finishing touches would be nice 👍

<CopyButtonTemplate
square
size="small"
textToCopy={`${window.location.host}/#/members/${candidate?.member.id}`}
Copy link
Contributor

Choose a reason for hiding this comment

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

So let's create a separate issue for handling links to candidates.

@p-sad
Copy link
Contributor Author

p-sad commented Oct 5, 2021

Closing because a github outage prevents the last 3 commits from showing up in the PR

@p-sad p-sad closed this Oct 5, 2021
@p-sad p-sad mentioned this pull request Oct 5, 2021
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