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

Add github light #49

Merged
merged 18 commits into from
Jun 22, 2023
Merged

Add github light #49

merged 18 commits into from
Jun 22, 2023

Conversation

steff456
Copy link
Contributor

@steff456 steff456 commented Apr 19, 2023

This PR is part of #40

For adding a new theme a new directory is created with the following structure,

├── theme_name
│   ├── src
│   │   ├── index.ts
│   ├── style
│   │   ├── index.css
│   │   ├── variables.css
│   ├── package.json
│   ├── tsconfig.json
│   ├── README.md

To test the new theme please follow the instructions in the updated readme.

A JupyterLab instance will start in your browser and you can select the pitaya smoothie theme, by clicking Settings > Theme > Github Light in the main menu.

Some screenshots on how it looks right now,

image

New theme checklist:

  • New directory with new package
  • Update the setup.py to include the new package
  • Create a README.md inside the new package with information about the color palette and resources
  • Update main README.md with new theme

@github-actions
Copy link

github-actions bot commented Apr 25, 2023

Binder 👈 Launch a binder notebook on branch Quansight-Labs/jupyterlab-accessible-themes/add-githublight
Comment updated on 2023-06-21T22:12:40.931Z

@steff456 steff456 marked this pull request as ready for review April 25, 2023 22:10
Copy link
Contributor

@gabalafou gabalafou left a comment

Choose a reason for hiding this comment

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

This is looking great! But I think you might need some fixes for the README (see inline comments)

packages/githublight/README.md Outdated Show resolved Hide resolved
packages/githublight/README.md Outdated Show resolved Hide resolved
packages/githublight/README.md Outdated Show resolved Hide resolved
packages/githublight/README.md Outdated Show resolved Hide resolved
packages/githublight/README.md Outdated Show resolved Hide resolved
packages/githublight/README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
packages/githublight/README.md Outdated Show resolved Hide resolved
packages/githublight/src/index.ts Outdated Show resolved Hide resolved
Copy link
Member

@trallard trallard left a comment

Choose a reason for hiding this comment

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

I have yet to check the extension on lab directly but I did a pass on the code here. There are several changes needed.

Also - does this theme need registering in the setup.py file like the previous theme added?

packages/githublight/README.md Outdated Show resolved Hide resolved
packages/githublight/README.md Outdated Show resolved Hide resolved
packages/githublight/README.md Outdated Show resolved Hide resolved
packages/githublight/package.json Outdated Show resolved Hide resolved
packages/githublight/package.json Outdated Show resolved Hide resolved
packages/githublight/style/variables.css Show resolved Hide resolved
packages/githublight/style/variables.css Show resolved Hide resolved
packages/githublight/style/variables.css Outdated Show resolved Hide resolved
packages/githublight/README.md Outdated Show resolved Hide resolved
@steff456
Copy link
Contributor Author

steff456 commented May 2, 2023

Also - does this theme need registering in the setup.py file like the previous theme added?

I think it does, but I'm not sure how it was working before 🤔

packages/githublight/README.md Outdated Show resolved Hide resolved
packages/githublight/package.json Outdated Show resolved Hide resolved
packages/githublight/style/variables.css Outdated Show resolved Hide resolved
Copy link
Member

@trallard trallard left a comment

Choose a reason for hiding this comment

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

Thanks, Stephannie I just did another pass on this PR.

Though on closer inspection, I noticed the index.css file for both themes is quite different in terms of what is being changed (buttons, tabs and the such) is this intentional and/or is there a rationale behind this?

packages/pitayasmoothie/README.md Outdated Show resolved Hide resolved
packages/githublight/README.md Outdated Show resolved Hide resolved
Copy link
Member

@trallard trallard left a comment

Choose a reason for hiding this comment

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

Thanks, Stephannie I just did another pass on this PR.

Though on closer inspection, I noticed the index.css file for both themes is quite different in terms of what is being changed (buttons, tabs and the such) is this intentional and/or is there a rationale behind this?

@trallard
Copy link
Member

Also I tried to install the package locally following the instructions in the README and was faced with this error

$ pip install -e .
DEPRECATION: Configuring installation scheme with distutils config files is deprecated and will no longer work in the near future. If you are using a Homebrew or Linuxbrew Python, please see discussion at https://github.com/Homebrew/homebrew-core/issues/76621
Obtaining file:///Users/tania/Documents/github/labs/jupyterlab-accessible-themes
  Installing build dependencies ... done
  Checking if build backend supports build_editable ... done
  Getting requirements to build wheel ... error
  error: subprocess-exited-with-error
  
  × Getting requirements to build wheel did not run successfully.
  │ exit code: 1
  ╰─> [22 lines of output]
      Traceback (most recent call last):
        File "/usr/local/lib/python3.9/site-packages/pip/_vendor/pyproject_hooks/_in_process/_in_process.py", line 353, in <module>
          main()
        File "/usr/local/lib/python3.9/site-packages/pip/_vendor/pyproject_hooks/_in_process/_in_process.py", line 335, in main
          json_out['return_val'] = hook(**hook_input['kwargs'])
        File "/usr/local/lib/python3.9/site-packages/pip/_vendor/pyproject_hooks/_in_process/_in_process.py", line 118, in get_requires_for_build_wheel
          return hook(config_settings)
        File "/usr/local/Cellar/[email protected]/3.9.16/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages/setuptools/build_meta.py", line 338, in get_requires_for_build_wheel
          return self._get_build_requires(config_settings, requirements=['wheel'])
        File "/usr/local/Cellar/[email protected]/3.9.16/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages/setuptools/build_meta.py", line 320, in _get_build_requires
          self.run_setup()
        File "/usr/local/Cellar/[email protected]/3.9.16/Frameworks/Python.framework/Versions/3.9/lib/python3.9/site-packages/setuptools/build_meta.py", line 335, in run_setup
          exec(code, locals())
        File "<string>", line 15, in <module>
        File "<string>", line 15, in <listcomp>
        File "/usr/local/Cellar/[email protected]/3.9.16/Frameworks/Python.framework/Versions/3.9/lib/python3.9/pathlib.py", line 1266, in read_text
          with self.open(mode='r', encoding=encoding, errors=errors) as f:
        File "/usr/local/Cellar/[email protected]/3.9.16/Frameworks/Python.framework/Versions/3.9/lib/python3.9/pathlib.py", line 1252, in open
          return io.open(self, mode, buffering, encoding, errors, newline,
        File "/usr/local/Cellar/[email protected]/3.9.16/Frameworks/Python.framework/Versions/3.9/lib/python3.9/pathlib.py", line 1120, in _opener
          return self._accessor.open(self, flags, mode)
      FileNotFoundError: [Errno 2] No such file or directory: 'jupyterlab_accessible_themes/labextensions/pitayasmoothie/package.json'
      [end of output]
  
  note: This error originates from a subprocess, and is likely not a problem with pip.
error: subprocess-exited-with-error

× Getting requirements to build wheel did not run successfully.
│ exit code: 1
╰─> See above for output.

note: This error originates from a subprocess, and is likely not a problem with pip.

@steff456
Copy link
Contributor Author

Though on closer inspection, I noticed the index.css file for both themes is quite different in terms of what is being changed (buttons, tabs and the such) is this intentional and/or is there a rationale behind this?

They are indeed different. The pitaya smoothie one I used the resources that are available in the repo, and trusted in the way that previous authors wanted to modify the interface. As I'm trying to get the github theme as close as I can to both the VScode extension and web interface, I needed to do some extra changes on how to color certain parts like buttons, tabs, etc.

My prediction is that the css will vary between themes depending on what I'm basing them on. So, for example the Github theme will change different things compared to the blinds or gotthard. But I would like to have the same css with color variants for each, so all the Github variants will share the same css (I'm testing how this looks with the dark colors right now).

There's infinite ways to change the colors of the UI, and I'm experimenting while creating each theme. But my goal is to reach a really similar look and feel with each theme.

@steff456
Copy link
Contributor Author

Also I tried to install the package locally following the instructions in the README and was faced with this error

Can you try again? I just updated the instructions, you need to build the node package and then you will be able to do the pip command. Let me know if that works!

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
packages/pitayasmoothie/README.md Show resolved Hide resolved
packages/githublight/README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@gabalafou gabalafou left a comment

Choose a reason for hiding this comment

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

Please update the PR description so that the installation instructions just say to follow the instructions in the updated readme.

Otherwise, it looks good to me. I got it to work. Switching themes in the UI is slick! Nice work!

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
@steff456
Copy link
Contributor Author

@trallard @gabalafou I already made the requested changes, please let me know if there's anything else this PR needs for merging

@trallard trallard merged commit b8e2f79 into main Jun 22, 2023
@trallard trallard deleted the add-githublight branch June 22, 2023 08:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done 💪🏾
Development

Successfully merging this pull request may close these issues.

3 participants