Skip to content

Export calendar improvements#9035

Merged
ahukkanen merged 5 commits intodecidim:developfrom
i-need-another-coffee:feature/export_calendar_improvements
Mar 29, 2022
Merged

Export calendar improvements#9035
ahukkanen merged 5 commits intodecidim:developfrom
i-need-another-coffee:feature/export_calendar_improvements

Conversation

@roxanaopr
Copy link
Copy Markdown
Contributor

@roxanaopr roxanaopr commented Mar 17, 2022 •
edited
Loading

This ticket needs to tackle the following :

  • (1) Calendar filter links not adding the filter params
  • (1) While filtering the export button does not open the pop-up ()
    If a user will filter the meetings listing in order to export the calendar only for specific meetings, the button will not be responding at all. In this case, we expect, after filtering is applied to have a new link that is specific to the filtered area.
  • (2) Add description text regarding the filters and add a copy button to the UI.

🎩 What? Why?

Please describe your pull request.

📌 Related Issues

(1) #8842
(2) #8625

Testing

Describe the best way to test or validate your PR.

📋 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.

📷 Scr

Screenshot from 2022-03-18 13-31-58
eenshots

♥️ Thank you!

@roxanaopr roxanaopr force-pushed the feature/export_calendar_improvements branch from e14d55f to 60687ce Compare March 17, 2022 16:34
@roxanaopr roxanaopr force-pushed the feature/export_calendar_improvements branch from 60687ce to 55e20ea Compare March 17, 2022 16:49
@roxanaopr roxanaopr force-pushed the feature/export_calendar_improvements branch from c238c1c to 74a89b6 Compare March 18, 2022 15:11
@roxanaopr roxanaopr marked this pull request as ready for review March 21, 2022 07:26
@roxanaopr roxanaopr requested a review from alecslupu March 21, 2022 07:26
alecslupu
alecslupu previously approved these changes Mar 21, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@alecslupu alecslupu left a comment

Choose a reason for hiding this comment

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

LGTM

@alecslupu alecslupu requested a review from ahukkanen March 21, 2022 08:34
ahukkanen
ahukkanen requested changes Mar 21, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@ahukkanen ahukkanen left a comment

Choose a reason for hiding this comment

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

Few small changes below. I'll test this once these are addressed.

@roxanaopr
Copy link
Copy Markdown
Contributor Author

roxanaopr commented Mar 24, 2022

@ahukkanen Can you please take a look after all the changes done?

ahukkanen
ahukkanen previously approved these changes Mar 28, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@ahukkanen ahukkanen left a comment

Choose a reason for hiding this comment

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

👍

@ahukkanen
Copy link
Copy Markdown
Contributor

ahukkanen commented Mar 28, 2022

Code-wise this is fine now but there are few more usability issues I noticed:

  1. When I filter the meetings, the export link does not update dynamically unless I reload the page with the filters in the URL
  2. The mouse cursor for the calendar URL is the "not allowed" cursor which can be confusing for the user:
    Not allowed cursor

In the share modal, we use the normal cursor:
Normal cursor

@roxanaopr
Copy link
Copy Markdown
Contributor Author

roxanaopr commented Mar 29, 2022

@ahukkanen Thank you for the feedback. Those 2 issues should be fixed now.

@roxanaopr roxanaopr force-pushed the feature/export_calendar_improvements branch from e2392a0 to dd2ff73 Compare March 29, 2022 13:19
ahukkanen
ahukkanen requested changes Mar 29, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@ahukkanen ahukkanen left a comment

Choose a reason for hiding this comment

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

Thanks @roxanaopr !

One small thing and I think we are there.

@roxanaopr roxanaopr force-pushed the feature/export_calendar_improvements branch from dd2ff73 to 789e452 Compare March 29, 2022 14:31
ahukkanen
ahukkanen approved these changes Mar 29, 2022
View reviewed changes
Copy link
Copy Markdown
Contributor

@ahukkanen ahukkanen left a comment

Choose a reason for hiding this comment

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

👍

@ahukkanen ahukkanen merged commit 3bfe7e1 into decidim:develop Mar 29, 2022
@roxanaopr
Copy link
Copy Markdown
Contributor Author

roxanaopr commented Mar 31, 2022

@andreslucena @ahukkanen
After the implementation of export calendar I saw that the link is not always imported in google calendar. For example: https://nightly.decidim.org/processes/provision-transfer/f/2/calendar?filter%5Bsearch_text_cont%5D=&filter%5Bwith_any_category%5D%5B%5D=&filter%5Bwith_any_date%5D%5B%5D=past&filter%5Bwith_any_type%5D%5B%5D=&filter%5Bwith_any_type%5D%5B%5D=online&filter%5Bwith_availability%5D= and I think this is because of the length of the url

@alecslupu alecslupu deleted the feature/export_calendar_improvements branch March 31, 2022 06:13
@alecslupu
Copy link
Copy Markdown
Contributor

alecslupu commented Mar 31, 2022

@andreslucena @ahukkanen After the implementation of export calendar I saw that the link is not always imported in google calendar. For example: https://nightly.decidim.org/processes/provision-transfer/f/2/calendar?filter%5Bsearch_text_cont%5D=&filter%5Bwith_any_category%5D%5B%5D=&filter%5Bwith_any_date%5D%5B%5D=past&filter%5Bwith_any_type%5D%5B%5D=&filter%5Bwith_any_type%5D%5B%5D=online&filter%5Bwith_availability%5D= and I think this is because of the length of the url

The issue reported has been fixed in : #9115

@alecslupu alecslupu mentioned this pull request Mar 31, 2022
Closed
12 tasks
entantoencuanto added a commit to PopulateTools/decidim that referenced this pull request Apr 5, 2022
@entantoencuanto
Merge branch 'chore/meetings_optimization' into pwa-staging
6d6adcf 
* chore/meetings_optimization:
  Add touch:true to Follow association with followable
  Fragment cache meetings partial in meetings index
  Bump minimist and node-forge (decidim#9131)
  Bump puma from 5.6.2 to 5.6.4 (decidim#9132)
  Add base URI to meta image URLs (decidim#9125)
  Make Decidim fully configurable via ENV vars part II (decidim#8990)
  Allow assembly admins to manage components in child assemblies (decidim#8955)
  Export calendar improvements (decidim#9035)
  Add reminders for publishing reports to meeting authors (decidim#8757)
  VAPID key generator availabe in core (decidim#9107)
  Dont add external link container inside editor (decidim#9095)
@ahukkanen ahukkanen mentioned this pull request Jun 1, 2022
Merged
12 tasks
This was referenced Aug 1, 2022
Closed
Closed
@ahukkanen ahukkanen mentioned this pull request Aug 15, 2022
Merged
@ahukkanen ahukkanen mentioned this pull request Apr 20, 2023
Merged
@alecslupu alecslupu added this to the 0.27.0 milestone Jul 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@andreslucena andreslucena andreslucena left review comments

@ahukkanen ahukkanen ahukkanen approved these changes

+1 more reviewer

@alecslupu alecslupu alecslupu left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

module: meetings

Projects

None yet

Milestone

0.27.0

Development

Successfully merging this pull request may close these issues.

4 participants

@roxanaopr @ahukkanen @alecslupu @andreslucena