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

Barceloneta based Classic UI theme #1516

Merged
merged 8 commits into from
Jul 13, 2023
Merged

Conversation

petschki
Copy link
Member

@petschki petschki commented Jul 3, 2023

No description provided.

@netlify
Copy link

netlify bot commented Jul 3, 2023

Deploy Preview for 6-docs-plone-org ready!

Name Link
🔨 Latest commit 63b3660
🔍 Latest deploy log https://app.netlify.com/sites/6-docs-plone-org/deploys/64ae70754e125e0008dec427
😎 Deploy Preview https://deploy-preview-1516--6-docs-plone-org.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@gogobd
Copy link

gogobd commented Jul 11, 2023

Hi, @petschki !

  • Maybe consider mentioning the version of npm you would recommend and that the generated theme addon needs to be installen (via Plone Panel).

Please mention that the file that should contain the font definitions is _variables.scss. I used a font other than the default font just to be able to see the change immediately:

$font-family-sans-serif:                    "Papyrus", "Fantasy";
$font-family-condensed:                     "Papyrus", "Fantasy"; //just on toolbar
$font-family-serif:                         "Papyrus", "Fantasy";
  • This $enabled-rounded: false; switch didn't work.

  • I was assuming that the changes mentioned in "Maps" would go into maps.scss, it's not mentioned but at the same time quite obvious i.e. most likely everyone's first guess.

@petschki petschki force-pushed the classic-ui-theming-barceloneta branch from 9af6515 to 53714cf Compare July 11, 2023 12:58
@petschki petschki marked this pull request as ready for review July 11, 2023 18:37
@petschki petschki force-pushed the classic-ui-theming-barceloneta branch from fd813ba to 0c10cdd Compare July 11, 2023 18:43
@stevepiercy
Copy link
Contributor

I just did a merge commit to bring it inline with main due to the failing docs test. We will see if it passes now.

Copy link
Contributor

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

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

Mostly a bunch of MyST, style guide, and English syntax and grammar fixes.

Would you please take a look at moving Node.js and nvm installation to the top of the page after the introduction, and see if there is something that can be reused from existing installation documentation? Thank you!

docs/classic-ui/theming/barceloneta.md Outdated Show resolved Hide resolved
docs/classic-ui/theming/barceloneta.md Outdated Show resolved Hide resolved
docs/classic-ui/theming/barceloneta.md Show resolved Hide resolved
docs/classic-ui/theming/barceloneta.md Outdated Show resolved Hide resolved
docs/classic-ui/theming/barceloneta.md Outdated Show resolved Hide resolved
docs/classic-ui/theming/barceloneta.md Outdated Show resolved Hide resolved
docs/classic-ui/theming/barceloneta.md Outdated Show resolved Hide resolved
docs/classic-ui/theming/barceloneta.md Outdated Show resolved Hide resolved
docs/classic-ui/theming/barceloneta.md Outdated Show resolved Hide resolved
docs/classic-ui/theming/barceloneta.md Outdated Show resolved Hide resolved
Co-Authored-By: Steve Piercy <web@stevepiercy.com>
@petschki petschki force-pushed the classic-ui-theming-barceloneta branch from 167d7fa to 6a19319 Compare July 12, 2023 08:21
@petschki
Copy link
Member Author

@stevepiercy thank you for your review. I've changed the doc and put the Node.js information at the top with a link to Install Plone from its packages. There I've found details on installing python, node, nvm ...

@petschki petschki force-pushed the classic-ui-theming-barceloneta branch from 9d377de to 12b956c Compare July 12, 2023 08:38
Break first subsection into two subsections.
s/addon/add-on
MyST and English syntax and grammar fixes.
Copy link
Contributor

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

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

I decided it would be easier to push a commit 63b3660 with my suggestions than go through a formal review. Feel free to change them. Thank you for your contribution. This is super helpful.



(classic-ui-theming-barceloneta-theme-package-label)=

## Theme package
## Create a Classic UI theme addon package
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
## Create a Classic UI theme addon package
## Create a Classic UI theme add-on package

@petschki
Copy link
Member Author

This is good to merge for me 🤗 @gogobd wanted to take a second look over this.

@petschki petschki merged commit 122b233 into 6.0 Jul 13, 2023
7 checks passed
@petschki petschki deleted the classic-ui-theming-barceloneta branch July 13, 2023 08:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants