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

Update @joeattardi/emoji-button to @picmo/popup-picker #9667

Merged
merged 23 commits into from
Oct 4, 2022

Conversation

ahukkanen
Copy link
Contributor

@ahukkanen ahukkanen commented Aug 9, 2022

🎩 What? Why?

As it often happens with NPM packages, @joeattardi/emoji-button is now deprecated as stated here:
https://emoji-button.js.org/

Fortunately the update is rather easy to @picmo/popup-picker which is what this PR does. As a bonus, this adds the possibility to translate the UI strings for the picker as pointed out at #9631.

Note that this is still a draft PR until the following issue is resolved at the dependency:
joeattardi/picmo#224

📌 Related Issues

Testing

  • Re-create the development app or replace its packages folder with the updated one
  • If you updated the packages folder manually
    • Run npm i within the development_app
    • Delete the previous assets by running rm -rf public/decidim-packs within the development_app
  • Start the development app
  • Log in
  • Find some place with a comments section (e.g. a proposal) and test out the updated emoji picker

📷 Screenshots

Before
Old emoji picker

After
New emoji picker

@ahukkanen ahukkanen added module: core type: feature PRs or issues that implement a new feature dependencies Pull requests that update a dependency file or issues that talk about updating dependencies labels Aug 9, 2022
@ahukkanen
Copy link
Contributor Author

ahukkanen commented Aug 9, 2022

Seems also we have similar validation issues generated as with the previous version.

We fixed this by disabling the error temporarily until it is fixed upstream (#8597) but it seems it is still not fixed for the HTML validator. We are still waiting for update of the HTML validator, the issue is already resolved at the CSS validator (see #8596).

The problem with the new version is that it contains many more of these values that we would need to ignore, so let's see if the HTML validator would be updated meanwhile we are waiting for the remaining issues to be resolved at @joeattardi/emoji-button.

There is also one additional CSS validation issue at @joeattardi/emoji-button that I reported here:
joeattardi/picmo#226

@joeattardi
Copy link

Hi all, PicMo maintainer here, I am working on the CSP issue and hope to have it resolved soon.

@ahukkanen ahukkanen marked this pull request as ready for review August 26, 2022 08:49
@ahukkanen
Copy link
Contributor Author

This is finally ready for review thanks to the fixes provided by @joeattardi. Many thanks!

Regarding the Nu validator bugs, unfortunately there is still no progress regarding those. In this PR, I am just ignoring the validator errors the same way as we used to do. The CSS generated by Picmo is valid but the problem is at the Nu validator. Related issues at the validator repositories:

@ahukkanen
Copy link
Contributor Author

The underlying issue with the HTML Nu validator seems to be finally fixed but we are still waiting for them to release a new version of the Docker container for those fixes to reach our CI flows.

More about this: #8596 (comment)

As we don't have class properties transform plugin for webpack,
use a constant instead to avoid "Missing class properties
transform" error.
@ahukkanen
Copy link
Contributor Author

Note for the reviewer:
The redesign changes at #9469 have broken the emoji picker positioning in the old layout as follows e.g. at the comments section:
Emoji picker toggle broken

This is not related to any changes in this PR.

If you enable redesign and go to conversations, the button is positioned correctly:
Emoji button position at redesign

Copy link
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.

Tried it locally, it works as expected. I see that we've also won i18n for this 👏🏽 👏🏽

@andreslucena andreslucena merged commit 069cb61 into decidim:develop Oct 4, 2022
@ahukkanen ahukkanen deleted the deps/picmo-emoji-button branch October 4, 2022 11:59
entantoencuanto added a commit that referenced this pull request Oct 5, 2022
* feature/redesign-turbo:
  Install turbo-rails
  Fix incorrect expecations after change of allowed file extensions (#9877)
  Update `@joeattardi/emoji-button` to `@picmo/popup-picker` (#9667)
  Address Crowdin feedback (#9678)
  Fix cryptic file validation errors (#9663)
  Fix disappearing sub-lists in rich text editors (#9867)
  Implement authorization data recovery for deleted users (#9463)
  Redesigned: admin bar (#9874)
eliegaboriau pushed a commit to eliegaboriau/decidim that referenced this pull request Oct 25, 2022
)

* Update `@joeattardi/emoji-button` to `@picmo/popup-picker`

* Update package-lock.json

* Update the design app packages and package-lock.json

* Fix unused i18n keys spec

* Remove unnecessary options

* Pass the `triggerElement` option to indicate the opening button

* Fix mobile positioning of the emoji picker

* Remove the overrides and solve the positioning using custom reference

In order not to override too much from the emoji popup picker,
create a specific reference element positioned exactly where we
want the popup picker to appear.

* Update `@picmo/popup-picker`

* Fix the screen jumping bug after upgrading `@picmo/popup-picker`

* Add aria text to the add emoji button

* Fix the CSS import for the popup-picker

* Fix emoji picker integration with the character counter

* Remove unnecessary CSS importing

* Fix the comment specs testing the visibility of the emoji section

* Update @picmo/popup-picker to 5.6.1

* Remove unnecessary customizations from emoji handler

After the latest fixes at picmo popup picker 5.6.1, the
customizations are no longer needed.

* Update teh ignored HTML CSS validation issues

After the switch to picmo, there are many more errors that we
need to ignore while we are waiting for the HTML validator to
be updated.

* Fix webpack compilation issue

As we don't have class properties transform plugin for webpack,
use a constant instead to avoid "Missing class properties
transform" error.

* Change emoji reference element to a block to fix accessibility issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file or issues that talk about updating dependencies module: core type: feature PRs or issues that implement a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Translation for Emojis Headings
3 participants