-
Notifications
You must be signed in to change notification settings - Fork 9.8k
[IMP] website: update building blocks #13568
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
Conversation
4a4f1ff
to
4ebefb4
Compare
4ebefb4
to
4231081
Compare
cb4b8ff
to
2a2a5f6
Compare
Hi @auva-odoo, could you please review this? We can also discuss this anytime, thank you so much! |
2a2a5f6
to
0fba09a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dear @masp-odoo, thank you for this PR!! I know it wasn't easy to reorganize the information and clean up so much content but I think you did a really good job overall!
I added a few comments in suggestions in my review (some we probably need to discuss IRL). In addition:
- Don't forget to resolve the conflict in the redirect file.
- Structure-wise: I think I would restructure the page a bit so things flow a bit more logically (to me at least?). For example, I would move the content of the "Inner content" + "Dynamic content" sections (to the "Add a building block" section (as sub-sections) and then the "Color" and "Layout" sections as sub-sections of the "Edit a building block" section. We can discuss this of course, it's just a suggestion! But I do think we can move things around a bit.
The structure would then look like this (see my individual comments as well for more info, I changed this screenshot several times 😅 )
- FYI, you could use the :scale: option to adapt the size of some screenshots if necessary (altough you will see I did suggest to get rid of a lot of screenshots)
- Could you please add the app name in the anchor names? Like this: app name/file name/anchor name
- Don't forget to add a descriptor for the icon names. I added a few comments here and there but I probably missed some, could you please check the whole doc?
Thank you again for your work! 🙏
content/applications/websites/website/web_design/building_blocks/2-types-of-blocks.png
Outdated
Show resolved
Hide resolved
content/applications/websites/website/web_design/building_blocks.rst
Outdated
Show resolved
Hide resolved
content/applications/websites/website/web_design/building_blocks.rst
Outdated
Show resolved
Hide resolved
content/applications/websites/website/web_design/building_blocks.rst
Outdated
Show resolved
Hide resolved
content/applications/websites/website/web_design/building_blocks.rst
Outdated
Show resolved
Hide resolved
content/applications/websites/website/web_design/building_blocks.rst
Outdated
Show resolved
Hide resolved
content/applications/websites/website/web_design/building_blocks.rst
Outdated
Show resolved
Hide resolved
content/applications/websites/website/web_design/building_blocks.rst
Outdated
Show resolved
Hide resolved
content/applications/websites/website/web_design/building_blocks.rst
Outdated
Show resolved
Hide resolved
content/applications/websites/website/web_design/building_blocks.rst
Outdated
Show resolved
Hide resolved
a4797d7
to
7a9dbdc
Compare
Hello again @auva-odoo, could you please review the changes ? Thanks a looooot! |
9bfbc21
to
aff7bd0
Compare
aff7bd0
to
e549380
Compare
e549380
to
bbeae93
Compare
taskid-4644908
bbeae93
to
7d0313b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you so much for your hard work @masp-odoo 💪 🙏 I force-pushed the changes we discussed 😉
@robodoo r+
Thank YOU @auva-odoo! |
taskid-4644908