Skip to content

DEV: Convert poll modals to new component-based API #22164

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
Merged
merged 6 commits into from
Jul 4, 2023
Merged

DEV: Convert poll modals to new component-based API #22164

merged 6 commits into from
Jul 4, 2023

Conversation

davidtaylorhq
Copy link
Member

(Requires #21304)

@davidtaylorhq davidtaylorhq added the js-modernization Tasks related to our JS modernization project, including the Ember 4.0+ upgrade label Jun 16, 2023
@davidtaylorhq davidtaylorhq force-pushed the poll-plugin-update branch 3 times, most recently from 99fd4f2 to 86b4c90 Compare June 19, 2023 16:45
@janzenisaac janzenisaac force-pushed the modal-named-outlet branch 3 times, most recently from 9e2d99d to e3c644b Compare June 29, 2023 18:30
Base automatically changed from modal-named-outlet to main July 3, 2023 09:51
@discoursebot
Copy link

This pull request has been mentioned on Discourse Meta. There might be relevant details there:

https://meta.discourse.org/t/converting-modals-from-legacy-controllers-to-new-dmodal-component-api/268057/1

janzenisaac
janzenisaac approved these changes Jul 3, 2023
View reviewed changes
plugins/poll/assets/javascripts/discourse/components/modal/poll-ui-builder.js Outdated
pollTitle = "";
pollOptions = null;
pollOptionsText = null;
pollTitle = null;
Copy link
Contributor

@janzenisaac janzenisaac Jul 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Seeing this is undefined by default, can we just do

Suggested change
pollTitle = null;
pollTitle;

Or just remove the explicit definition all together?

plugins/poll/assets/javascripts/discourse/components/modal/poll-ui-builder.js Outdated
Comment on lines 30 to 31
pollGroups = null;
pollAutoClose = null;
Copy link
Contributor

Choose a reason for hiding this comment

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

Dito

@davidtaylorhq davidtaylorhq merged commit 773e198 into main Jul 4, 2023
@davidtaylorhq davidtaylorhq deleted the poll-plugin-update branch July 4, 2023 14:25
CvX added a commit that referenced this pull request Aug 3, 2023
@CvX
FIX: Make poll breakdown modal closable again
a8b8698 
Regressed in #22164
@CvX CvX mentioned this pull request Aug 3, 2023
Merged
CvX added a commit that referenced this pull request Aug 3, 2023
@CvX
FIX: Make poll breakdown modal closable again
465fa5d 
Regressed in #22164
@CvX CvX mentioned this pull request Aug 3, 2023
Merged
CvX added a commit that referenced this pull request Aug 3, 2023
@CvX
FIX: Make poll breakdown modal closable again (#22953)
73ee3c1 
Regressed in #22164
CvX added a commit that referenced this pull request Aug 3, 2023
@CvX
FIX: Make poll breakdown modal closable again (#22952)
2b04301 
Regressed in #22164
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CvX CvX CvX approved these changes

@janzenisaac janzenisaac janzenisaac approved these changes

Assignees
No one assigned
Labels
js-modernization Tasks related to our JS modernization project, including the Ember 4.0+ upgrade
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@davidtaylorhq @discoursebot @CvX @janzenisaac