Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Sketchbook explorer #344

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

Merged
fstasi merged 1 commit into main from atl-1217--sketchbook-view-menus
Jul 6, 2021
Merged

Sketchbook explorer #344

fstasi merged 1 commit into main from atl-1217--sketchbook-view-menus
Jul 6, 2021

Conversation

@fstasi
Copy link
Contributor

@fstasi fstasi commented Apr 28, 2021 •
edited
Loading

Motivation

Local and remote sketchbook functionalities. Feature is documented here

Change description

  • adding local sketchbook widget
  • adding remote sketchbook widget
  • updating preferences accordingly

Reviewer checklist

  • PR address a single concern.
  • PR title and description are properly filled.
  • History is clean, commit messages are meaningful.

@fstasi fstasi requested a review from kittaakos April 28, 2021 07:40
Copy link

CLAassistant commented Apr 28, 2021 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

Please revert the indentation changes, and then I look into the PR.

@fstasi fstasi force-pushed the atl-1217--sketchbook-view-menus branch from efecad3 to e6d1c7d Compare April 28, 2021 07:51
Copy link
Contributor Author

fstasi commented Apr 28, 2021

Please revert the indentation changes, and then I look into the PR.

done... don't know why formatter put a 2 space tab size on that file 🤷

Copy link
Contributor

@kittaakos kittaakos left a comment

Choose a reason for hiding this comment

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

I cannot add custom icons. It does not work anymore.

@fstasi fstasi force-pushed the atl-1217--sketchbook-view-menus branch from e6d1c7d to 080c396 Compare April 28, 2021 10:55
@fstasi fstasi requested a review from kittaakos April 28, 2021 12:31
@fstasi fstasi force-pushed the atl-1217--sketchbook-view-menus branch 3 times, most recently from 645333b to eb66f4d Compare April 29, 2021 14:45
@fstasi fstasi changed the base branch from atl-1217--sketchbook-view to main April 29, 2021 14:46
@fstasi fstasi reopened this Apr 29, 2021
@fstasi fstasi force-pushed the atl-1217--sketchbook-view-menus branch from eb66f4d to e1a8647 Compare April 30, 2021 13:02
Copy link

ubidefeo commented May 1, 2021

I took some time to review the behaviour and found a few things I don't like.
Taking notes and reporting back on Monday

Copy link

@ubidefeo ubidefeo left a comment •
edited
Loading

Choose a reason for hiding this comment

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

@fstasi here's my feedback on the tests

Implementation

  • Sketchbook explorer tab icon not visible at launch: make sure it shows up
  • Hidden (.SOMETHING) files and folders should not be visible
  • Double-click on a sketch main folder opens in a new window, but before it toggles expand/contract view. Maybe the expand feature should only happen on the arrow
  • When a file external to the sketch is clicked (preview mode) it occupies a tab next to the Sketch's files. Closing the Sketch and reopening it preserves this tab. This behaviour should be eliminated: opening a Sketch only opens its files

Design

  • the Arduino Logo as an icon for .ino files is identifiable but not looking good. The .ino file icon is the same assigned to the Sketch container

@fstasi fstasi force-pushed the atl-1217--sketchbook-view-menus branch 2 times, most recently from eda47cb to e56f5cd Compare May 12, 2021 09:39
Copy link
Contributor Author

fstasi commented May 12, 2021 •
edited
Loading 

✅ Sketchbook explorer tab icon not visible at launch: make sure it shows up
✅ Hidden (`.SOMETHING`) files and folders should not be visible
✅ Double-click on a sketch main folder opens in a new window, but before it toggles expand/contract view. Maybe the expand feature should only happen on the arrow
🚯 When a file external to the sketch is clicked (preview mode) it occupies a tab next to the Sketch's files. Closing the Sketch and reopening it preserves this tab. This behaviour should be eliminated: opening a Sketch only opens its files

@ubidefeo Last comment become this task

@fstasi fstasi requested a review from ubidefeo May 12, 2021 09:42
@fstasi fstasi force-pushed the atl-1217--sketchbook-view-menus branch from e56f5cd to 0295cfc Compare May 12, 2021 09:45
Copy link

@ubidefeo ubidefeo left a comment

Choose a reason for hiding this comment

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

all works except
*the Arduino Logo as an icon for .ino files is identifiable but not looking good. The .ino file icon is the same assigned to the Sketch container

please replace the .ino file icon with the one used for the Sketch container:
image

@fstasi fstasi force-pushed the atl-1217--sketchbook-view-menus branch from 0295cfc to 77ba0cb Compare May 18, 2021 08:43
Copy link
Contributor Author

fstasi commented May 18, 2021

all works except
*the Arduino Logo as an icon for .ino files is identifiable but not looking good. The .ino file icon is the same assigned to the Sketch container

please replace the .ino file icon with the one used for the Sketch container:
image

done!

@fstasi fstasi force-pushed the atl-1217--sketchbook-view-menus branch 3 times, most recently from 6239a58 to d7c7898 Compare May 24, 2021 16:29
@fstasi fstasi force-pushed the atl-1217--sketchbook-view-menus branch from d7c7898 to 3bcdc59 Compare May 26, 2021 16:19
@fstasi fstasi force-pushed the atl-1217--sketchbook-view-menus branch 5 times, most recently from 0d4068f to cdcec92 Compare June 1, 2021 10:51
@fstasi fstasi force-pushed the atl-1217--sketchbook-view-menus branch from cdcec92 to b9ad341 Compare June 3, 2021 17:07
@fstasi fstasi force-pushed the atl-1217--sketchbook-view-menus branch 5 times, most recently from e358a56 to b8f87dc Compare June 22, 2021 15:27
@fstasi fstasi force-pushed the atl-1217--sketchbook-view-menus branch 2 times, most recently from 9c476c2 to 8b0d663 Compare June 29, 2021 14:34
@fstasi fstasi force-pushed the atl-1217--sketchbook-view-menus branch 2 times, most recently from 5bc945e to 69d7f29 Compare July 1, 2021 13:42
@fstasi fstasi force-pushed the atl-1217--sketchbook-view-menus branch from 4956080 to cfaf905 Compare July 6, 2021 10:59
@fstasi fstasi merged commit 4c536ec into main Jul 6, 2021
@fstasi fstasi deleted the atl-1217--sketchbook-view-menus branch July 6, 2021 12:58
@per1234 per1234 added topic: code Related to content of the project itself type: enhancement Proposed improvement labels Oct 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ubidefeo ubidefeo ubidefeo requested changes

@kittaakos kittaakos Awaiting requested review from kittaakos

+1 more reviewer

@rsora rsora rsora approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

topic: code Related to content of the project itself type: enhancement Proposed improvement

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /