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.

Sign up for GitHub

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

Jump to bottom

update layoutcomponents page #10398

Merged
merged 6 commits into from May 26, 2023
Merged

update layoutcomponents page #10398

merged 6 commits into from May 26, 2023

Conversation

szaimen
Copy link
Contributor

@szaimen szaimen commented May 16, 2023 •
edited

Address part of #10392

TODO:

  • Update all screenshots
  • update links
  • move links to top?
  • add links to penpot
  • address reviews

@szaimen
update layoutcomponents page
b73b994 
Signed-off-by: Simon L <szaimen@e.mail.de>
@szaimen szaimen added this to the Nextcloud 27 milestone May 16, 2023
@szaimen
Move links up and add a few links
925259b 
Signed-off-by: Simon L <szaimen@e.mail.de>
@szaimen szaimen marked this pull request as ready for review May 23, 2023 14:41
@szaimen szaimen requested a review from nimishavijay May 23, 2023 14:41
@blizzz blizzz modified the milestones: Nextcloud 27, Nextcloud 28 May 23, 2023
@szaimen
add penpot links
a44fa27 
Signed-off-by: Simon L <szaimen@e.mail.de>
@szaimen
update penpo links
cbfdcc5 
Signed-off-by: Simon L <szaimen@e.mail.de>
@nimishavijay
Copy link
Member

For the test accounts another thing you could do is to just change the text in the inspector and then take the screenshot :)

jancborchardt
jancborchardt reviewed May 24, 2023
View reviewed changes
developer_manual/design/layoutcomponents.rst
@@ -95,7 +95,7 @@ Settings

`Settings Vue component <https://nextcloud-vue-components.netlify.app/#/Components/App%20containers/NcAppNavigation?id=ncappnavigationsettings>`_.

`Penpot Modals <https://design.penpot.app/#/view/db3839da-807b-8052-8002-576401e9a375?page-id=3f784c86-6c27-80c6-8002-6ab157b6bd27&section=interactions&index=12&share-id=88d87192-d205-8007-8002-8d8d21a0dcca>`_
`Penpot modals <https://design.penpot.app/#/view/db3839da-807b-8052-8002-576401e9a375?page-id=3f784c86-6c27-80c6-8002-6ab157b6bd27&section=interactions&index=12&share-id=11fde340-21f4-802e-8002-8d8d305e7ab5>`_
Copy link
Member

Choose a reason for hiding this comment

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

@nimishavijay on the modals page, the link to the docs doesn’t seem clickable.
Do you think we should also label the modals according to size in Penpot? But probably it’s enough in the docs.

Copy link
Member

Choose a reason for hiding this comment

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

Ah you're right. The links didn't save for some reason? I will check again and fix it.

We can add labels for sizes on Penpot too. It would be similar to the labels for the Checkbox/radio/switch component :)

jancborchardt
jancborchardt reviewed May 24, 2023
View reviewed changes
developer_manual/design/layoutcomponents.rst Outdated
@@ -115,7 +115,7 @@ List item

`List item Vue component <https://nextcloud-vue-components.netlify.app/#/Components/NcListItems>`_.

`Penpot List item <https://design.penpot.app/#/view/db3839da-807b-8052-8002-576401e9a375?page-id=3f784c86-6c27-80c6-8002-6ab157b6bd27&section=interactions&index=9&share-id=88d87192-d205-8007-8002-8d8d21a0dcca>`_
`Penpot List item <https://design.penpot.app/#/view/db3839da-807b-8052-8002-576401e9a375?page-id=3f784c86-6c27-80c6-8002-6ab157b6bd27&section=interactions&index=9&share-id=11fde340-21f4-802e-8002-8d8d305e7ab5>`_
Copy link
Member

Choose a reason for hiding this comment

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

@szaimen detail:

Suggested change
`Penpot List item <https://design.penpot.app/#/view/db3839da-807b-8052-8002-576401e9a375?page-id=3f784c86-6c27-80c6-8002-6ab157b6bd27&section=interactions&index=9&share-id=11fde340-21f4-802e-8002-8d8d305e7ab5>`_
`Penpot list items <https://design.penpot.app/#/view/db3839da-807b-8052-8002-576401e9a375?page-id=3f784c86-6c27-80c6-8002-6ab157b6bd27&section=interactions&index=9&share-id=11fde340-21f4-802e-8002-8d8d305e7ab5>`_

And @nimishavijay the link back to the docs page doesn’t seem to work here either?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The link is fixed already

jancborchardt
jancborchardt reviewed May 24, 2023
View reviewed changes
developer_manual/design/layoutcomponents.rst
@@ -162,7 +162,7 @@ Sidebar

`Sidebar Vue component <https://nextcloud-vue-components.netlify.app/#/Components/App%20containers/NcAppSidebar?id=ncappsidebar-1>`_.

`Penpot Sidebar <https://design.penpot.app/#/view/db3839da-807b-8052-8002-576401e9a375?page-id=3f784c86-6c27-80c6-8002-6ab157b6bd27&section=interactions&index=11&share-id=88d87192-d205-8007-8002-8d8d21a0dcca>`_
`Penpot sidebar <https://design.penpot.app/#/view/db3839da-807b-8052-8002-576401e9a375?page-id=3f784c86-6c27-80c6-8002-6ab157b6bd27&section=interactions&index=11&share-id=11fde340-21f4-802e-8002-8d8d305e7ab5>`_
Copy link
Member

Choose a reason for hiding this comment

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

@nimishavijay same here for the documentation link

jancborchardt
jancborchardt approved these changes May 24, 2023
View reviewed changes
Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

Only one little detail @szaimen otherwise all good and all links tested in private mode. :)

@szaimen
fix typo
6162ce0 
Signed-off-by: Simon L <szaimen@e.mail.de>
@szaimen
Copy link
Contributor Author

szaimen commented May 24, 2023

Can we block this with a review so that we don't merge this until I have not updated the problematic screenshots? :)

@jancborchardt jancborchardt self-requested a review May 24, 2023 15:03
jancborchardt
jancborchardt requested changes May 24, 2023
View reviewed changes
Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

I hereby request update of the problematic screenshots @szaimen! :D

@szaimen
update problematig pictures
8a55557 
Signed-off-by: Simon L <szaimen@e.mail.de>
jancborchardt
jancborchardt reviewed May 25, 2023
View reviewed changes
developer_manual/images/sidebar-sharing.png
Copy link
Member

Choose a reason for hiding this comment

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

Regarding the sidebar, the new sidebar tabs component design is already in and we could include it, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is how it currently looks on tech-preview so not sure if it is already in? I think this was postponed for vue 8 which is not release yet...

jancborchardt
jancborchardt reviewed May 25, 2023
View reviewed changes
developer_manual/images/list-item.gif
Copy link
Member

Choose a reason for hiding this comment

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

This will have to be updated soon as per nextcloud-libraries/nextcloud-vue#4158

jancborchardt
jancborchardt approved these changes May 25, 2023
View reviewed changes
Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

Looks good! @szaimen in the spirit of step-by-step improvements we can do the rest in a follow-up? :)

@szaimen
Copy link
Contributor Author

szaimen commented May 26, 2023

Looks good! @szaimen in the spirit of step-by-step improvements we can do the rest in a follow-up? :)

Yes, lets do that :)

@szaimen szaimen merged commit e36ae26 into master May 26, 2023
8 checks passed
@szaimen szaimen deleted the enh/noid/update-layout-components branch May 26, 2023 07:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jancborchardt jancborchardt jancborchardt approved these changes

@nimishavijay nimishavijay Awaiting requested review from nimishavijay

@marcoambrosini marcoambrosini Awaiting requested review from marcoambrosini

Assignees
No one assigned
Labels
3. to review
Projects
None yet
Milestone
Nextcloud 28
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@szaimen @nimishavijay @jancborchardt @blizzz