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

Standardize Property Inspector placeholder according to TOC #16566

Merged

Conversation

joaopalmeiro
Copy link
Contributor

@joaopalmeiro joaopalmeiro commented Jul 9, 2024

Hi! 👋

References

Closes #14235

Code changes

The Property Inspector placeholder styling was adapted based on the Table of Contents placeholder as well as the HTML elements that make up it to take advantage of <p> styles and for consistency.

Let me know your feedback, please!

User-facing changes

The Property Inspector placeholder is now visually and element-wise similar to the Table of Contents placeholder:

Screenshot 2024-07-09 at 18 25 33

Backwards-incompatible changes

Yes, given that translatable strings have changed.

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
Copy link

Thanks for making a pull request to jupyterlab!
To try out this branch on binder, follow this link: Binder

@github-actions github-actions bot added tag:CSS For general CSS related issues and pecadilloes pkg:property-inspector labels Jul 9, 2024
@krassowski
Copy link
Member

I would go ahead and change the text to something more useful and following the convention as in TOC, e.g.:

No properties
Property inspector allows to view and edit properties of a selected notebook.

Unless you would like this PR to be backported to 4.2.x as-is and then make a new PR with the updated text (to avoid breaking experience of users who use language packs we do not change translatable strings in the main parts of the UI in backports).

@krassowski krassowski added the bug label Jul 10, 2024
@joaopalmeiro
Copy link
Contributor Author

joaopalmeiro commented Jul 10, 2024

I can implement this change too, no problem, @krassowski!

Personally, I have no need for this PR to be backported to JupyterLab 4.2 (for context, we are working with JupyterLab 4.1 and the plan is to upgrade to 4.3 when possible).

Let me know if I can make the change you suggested or if you would prefer to proceed another way.

Btw, after adding/changing a translatable string, do I need to open a PR on the https://github.com/jupyterlab/language-packs side?

@krassowski
Copy link
Member

Let me know if I can make the change you suggested or if you would prefer to proceed another way.

Sure, let's target 4.3 with this PR and add the text changes here.

No, no need to open PR on language packs, it is all automated.

@krassowski krassowski added this to the 4.3.0 milestone Jul 10, 2024
@joaopalmeiro joaopalmeiro marked this pull request as draft July 10, 2024 10:14

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature.
… Inspector
@joaopalmeiro joaopalmeiro marked this pull request as ready for review July 10, 2024 10:47
@joaopalmeiro
Copy link
Contributor Author

@krassowski, the PR is ready for a new review!

I changed the text you suggested implementing a bit to, from my perspective, make it more consistent with the Table of Contents one, that is, the header in Title Case and the text starting with "The":

Screenshot 2024-07-10 at 11 46 23

Let me know your thoughts, please!

Copy link
Member

@fcollonval fcollonval left a comment

Choose a reason for hiding this comment

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

@krassowski krassowski merged commit 6d4e299 into jupyterlab:main Jul 10, 2024
83 checks passed
@joaopalmeiro joaopalmeiro deleted the update-property-inspector-placeholder branch July 10, 2024 17:49
@krassowski krassowski mentioned this pull request Aug 11, 2024
18 tasks
ImpSy pushed a commit to spotinst/jupyterlab that referenced this pull request Jan 7, 2025
…ab#16566)

* Standardize Property Inspector placeholder according to TOC

* Add placeholder headline and update placeholder text for the Property Inspector

* Update snapshot

---------

Co-authored-by: krassowski <5832902+krassowski@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Placeholder for property inspector should be consistent with TOC
3 participants