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

[#8161] poll redesign #5630

Merged
merged 1 commit into from
Jun 27, 2024
Merged

[#8161] poll redesign #5630

merged 1 commit into from
Jun 27, 2024

Conversation

hom3mad3
Copy link
Contributor

@hom3mad3 hom3mad3 commented Jun 19, 2024

Describe your changes
Style overwrites for the poll module
Dependencies: liqd/adhocracy4#1640

testing: create a poll module (i.e https://meinberlin-design-dev.liqd.net/projekte/digital-participation-poll-for-mitte/)

  • Need to clarify with design first, as some of the original design in zeplin was not trivial to implement and we will end up having to compromise. update: approved by design
  • color variables need to be refactored in a different PR

polls

Tasks

  • PR name contains story or task reference
  • Steps to recreate and test the changes
  • Tests (including n+1 and django_assert_num_queries where applicable)
  • Changelog

@hom3mad3 hom3mad3 force-pushed the mr-2024-06-poll-redesign branch 2 times, most recently from 3ba0aac to 17759eb Compare June 20, 2024 07:37
@hom3mad3 hom3mad3 requested review from goapunk and m4ra June 20, 2024 07:39
Copy link
Contributor

@goapunk goapunk left a comment

Choose a reason for hiding this comment

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

code looks good but there are some broken elements (will leave them for @mcastro-lqd)

@@ -14,25 +14,25 @@ $warning: #f49e00 !default;
$danger: #a72b1e !default;

$body-bg: #fff !default;
$bg-secondary: #f2f2f2 !default;
$bg-secondary: #f5f5f5 !default; // updated
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need the comment here?

Copy link
Contributor Author

@hom3mad3 hom3mad3 Jun 24, 2024

Choose a reason for hiding this comment

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

its a convention that was started with the redesign, but makes no sense, i kinda gave up once i realized the state of the variables file. i will clean it up

Copy link
Contributor

Choose a reason for hiding this comment

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

will leave it up to you, if it helps feel free to keep it

@hom3mad3
Copy link
Contributor Author

code looks good but there are some broken elements (will leave them for @mcastro-lqd)

thanks, as mentioned already checked design with @mcastro-lqd but will let him double check once its in staging

@goapunk
Copy link
Contributor

goapunk commented Jun 26, 2024

@hom3mad3 only needs an a4 update and then we can merge it

@hom3mad3
Copy link
Contributor Author

@hom3mad3 only needs an a4 update and then we can merge it

now there are unrelated failing tests :/ not sure what is going on

@goapunk
Copy link
Contributor

goapunk commented Jun 26, 2024

@hom3mad3 only needs an a4 update and then we can merge it

now there are unrelated failing tests :/ not sure what is going on

hmm, maybe the something changed since the last update, there were some fixes to the comments, will have a look tomorrow

@goapunk goapunk enabled auto-merge (rebase) June 27, 2024 14:35
@goapunk goapunk disabled auto-merge June 27, 2024 14:35
@goapunk goapunk enabled auto-merge (rebase) June 27, 2024 14:35
@goapunk goapunk merged commit 657d849 into dev Jun 27, 2024
2 checks passed
@goapunk goapunk deleted the mr-2024-06-poll-redesign branch June 27, 2024 14:43
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.

None yet

2 participants