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

A11y Addons control panel #6345

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

deodorhunter
Copy link
Contributor

@deodorhunter deodorhunter commented Sep 27, 2024

This PR overhauls current Addons Control panel UI using react-aria-components and solves #5142.
All accordions removed, UI is now akin to Plone Classic.
Aria pattern used: ARIA grid pattern through GridList component

Changelog:

  • Written in Typescript leveraging @plone/types
  • Component UI overhauled and code splitted, really minor changes to existing logic (types mostly)
  • Updated tests and added new tests and related mocks/fixtures/snapshots for Cypress and Jest/Testing Library.
  • Updated some Addons related types in @plone/types package
  • Added stylesheet and imported it directly in the component, css splitting and lazy loading works as expected. Will move styles in less theme files if deemed better/necessary
  • Translations

UI Preview

image
image

@deodorhunter deodorhunter self-assigned this Sep 27, 2024
@deodorhunter deodorhunter linked an issue Sep 27, 2024 that may be closed by this pull request
Copy link

netlify bot commented Sep 27, 2024

Deploy Preview for plone-components canceled.

Name Link
🔨 Latest commit 54a44e0
🔍 Latest deploy log https://app.netlify.com/sites/plone-components/deploys/66fa9a6976291f0008b96cd7

padding: 2rem 1.5rem;
}

#page-addons .btn-primary {
Copy link
Sponsor Member

Choose a reason for hiding this comment

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

I am not a fan of this styling of button variants where we introduce css overrides.
Since we are starting over and making use of css vars we should have a button class
and variant classes that where possible just modifies the vars.
Have a look at this article where this designer shows how they made their button
component.
https://piccalil.li/blog/how-i-build-a-button-component/#variants

I feel like we still need to adopt a styling methodology and try to use it in the new
components if we are now at the totally breaking future phase.
Will have another look at the plone.contents efforts and plone.components and
come back with further observations for the future.

Copy link
Contributor

Choose a reason for hiding this comment

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

@ichim-david it's out of the scope of this PR to go in the direction of the new theming system. Since we don't know how that will be yet, it sounds like a waste of time for me to make it up in this specific PR. Moreover, Victor and I specifically told everybody working on reimplementing these panels and other views to avoid adding plone/components and such because that's not coming in volto 18 while these fixes should be there if possible.

Copy link
Contributor Author

@deodorhunter deodorhunter Sep 30, 2024

Choose a reason for hiding this comment

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

@ichim-david I understand your concerns, and this kind of "custom temporary styling" is not my preferred style either.
I would have gone with a cva/cva like solution (https://cva.style/docs) if we had a theming convention (at least on paper), but alas that's still a long way in the future from what I understand.
As @pnicolli said, we had a talk about this during the sprint, and decided to avoid using plone/components and unified theming like quanta styles.

After some thinking, a possible edge case issue with my initial implementation of selector naming came to mind: people using custom cms-ui styles, like injecting bootstrap or a custom stylesheet with the same selector naming convention. It would have caused inconsistencies and hassles, so I changed the button selector names to reflect the scope of action of that button (install/install upgrade and uninstall).

@deodorhunter
Copy link
Contributor Author

Update
This pr is now complete with all missing translations in all supported languages. Got some help from our well known GPT buddy, as I would have hated leaving blanks for other languages.
Haven't seen strange/unusual translations during a quick cross check with Translate and Deepl, but you never know...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

a11y - site setup - Addon-ons - cms ui
3 participants