Skip to content

Fix meetings form embed type visibility#8602

Merged
andreslucena merged 6 commits intodevelopfrom
fix/meetings_form_embed_type_visibility
Jan 5, 2022
Merged

Fix meetings form embed type visibility#8602
andreslucena merged 6 commits intodevelopfrom
fix/meetings_form_embed_type_visibility

Conversation

@entantoencuanto
Copy link
Copy Markdown
Contributor

@entantoencuanto entantoencuanto commented Dec 10, 2021

🎩 What? Why?

Please describe your pull request.

📌 Related Issues

Link your PR to an issue

Testing

This PR:

  • Fixes the visibility of iframe embed type select depending on meeting type
  • Allows participants to set iframe access level
  • Fixes the visibility of iframe access level depending on embed type options
  • Remove blank option of embed type select both for participants and admins

📋 Checklist

🚨 Please review the guidelines for contributing to this repository.

  • CONSIDER adding a unit test if your PR resolves an issue.
  • ✔️ DO check open PR's to avoid duplicates.
  • ✔️ DO keep pull requests small so they can be easily reviewed.
  • ✔️ DO build locally before pushing.
  • ✔️ DO make sure tests pass.
  • ✔️ DO make sure any new changes are documented in docs/.
  • ✔️ DO add and modify seeds if necessary.
  • ✔️ DO add CHANGELOG upgrade notes if required.
  • ✔️ DO add to GraphQL API if there are new public fields.
  • ✔️ DO add link to MetaDecidim if it's a new feature.
  • AVOID breaking the continuous integration build.
  • AVOID making significant changes to the overall architecture.

📷 Screenshots

Please add screenshots of the changes you're proposing
Description

♥️ Thank you!

@entantoencuanto entantoencuanto force-pushed the fix/meetings_form_embed_type_visibility branch 4 times, most recently from e4a08d8 to 29f2527 Compare December 10, 2021 20:41
@entantoencuanto entantoencuanto force-pushed the fix/meetings_form_embed_type_visibility branch from 29f2527 to 7d730c7 Compare December 10, 2021 21:26
entantoencuanto added a commit to PopulateTools/decidim-populate-dev that referenced this pull request Dec 10, 2021
This includes the PR decidim/decidim#8602
recently rebased on develop
@entantoencuanto entantoencuanto marked this pull request as ready for review December 10, 2021 22:11
@entantoencuanto
Copy link
Copy Markdown
Contributor Author

Hi, @decidim/product , you can review this on populate staging

@andreslucena andreslucena changed the title Fix/meetings form embed type visibility Fix meetings form embed type visibility Dec 13, 2021
@andreslucena
Copy link
Copy Markdown
Member

@entantoencuanto I think there's a problem with this form. I can reproduce it locally and in https://decidim.populate.tools (https://decidim.populate.tools/processes/subprova/f/256/meetings/new)

The problem is that I see the "Iframe access level" when I go to the meetings' creation form, even when I didn't select any meeting type

image

What I expect to happen is to only see it when the meeting is of type "online" or "both"
I can reproduce this in Chromium (Versió 96.0.4664.45 (Compilació oficial) snap (64 bits)) and in Firefox (94.0 (64-bit))
Also, I see some exceptions in the web developer JS console (I don't know if it's related or no to this issue)

Firefox
image

Chromium
image

@andreslucena andreslucena added module: meetings project: online-meetings Barcelona City Council contract labels Dec 13, 2021
@entantoencuanto
Copy link
Copy Markdown
Contributor Author

entantoencuanto commented Dec 14, 2021

Hi, @andreslucena, I've deployed again rebuilding assets. It seems that the assets were not being compiled properly and the javascript responsible of hiding the iframe access level select was not present. In https://decidim.populate.tools/processes/subprova/f/256/meetings/new the select is now hidden.

Locally I don't have the same issue as you, could you rebuild the development_app again from this branch and check if assets are compiled correctly?

The exception in the console seems to not be related with this PR, in develop branch I see the same error.

@andreslucena
Copy link
Copy Markdown
Member

Locally I don't have the same issue as you, could you rebuild the development_app again from this branch and check if assets are compiled correctly?

Now its working

The exception in the console seems to not be related with this PR, in develop branch I see the same error.

Ok, reported at #8640

@andreslucena
Copy link
Copy Markdown
Member

andreslucena commented Dec 21, 2021

@entantoencuanto I've reviewed it. It's looking good 😄

Just a small change: as I mentioned in #8570, participants can't create Polls (at least for now), so it doesn't make sense to have the option "Open in live event page (with optional polls)":

Also, as participants can't create polls (#8065), then it doesn't make sense to have the "Iframe embed type" of "Open in live event page (with optional polls)"

It's also in the Acceptance criteria:

  • Given that I'm a participant creating or editing a meeting
    when I select the "Online" or "Both" types,
    then in "Iframe embed type" I have only the options "None", "Embed in meeting page" and "Open in a new tab"

Can you remove that option only for participants 🙏🏽? Thanks

Copy link
Copy Markdown
Member

@andreslucena andreslucena left a comment

Choose a reason for hiding this comment

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

As I mentioned in my last comment, can we drop the "live event" option from the meetings participant form 🙏🏽 ?

@entantoencuanto
Copy link
Copy Markdown
Contributor Author

Changes added, ready to check, @andreslucena

cc/ @decidim/product

@andreslucena andreslucena merged commit e70a985 into develop Jan 5, 2022
@andreslucena andreslucena deleted the fix/meetings_form_embed_type_visibility branch January 5, 2022 08:48
@andreslucena andreslucena added the type: fix PRs that implement a fix for a bug label Jan 10, 2022
@alecslupu alecslupu added this to the 0.26.0 milestone Jul 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

in-review module: meetings project: online-meetings Barcelona City Council contract type: fix PRs that implement a fix for a bug

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Online meetings with meetings created by participants

3 participants