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

[CLOSED] Initial Quick Docs implementation #3239

Open
core-ai-bot opened this issue Aug 29, 2021 · 10 comments
Open

[CLOSED] Initial Quick Docs implementation #3239

core-ai-bot opened this issue Aug 29, 2021 · 10 comments

Comments

@core-ai-bot
Copy link
Member

Issue by peterflynn
Tuesday Apr 16, 2013 at 05:51 GMT
Originally opened as adobe/brackets#3449


This introduces:

  • In core: a "Quick Docs" command and an extension point for it, paralleling Quick Edit
  • In an extension: a specific Quick Docs provider for the Web Platform Docs CSS dataset

This doesn't include final XD layout/styles yet, so initial feedback should focus on the JS parts.


peterflynn included the following code: https://github.com/adobe/brackets/pull/3449/commits

@core-ai-bot
Copy link
Member Author

Comment by jasonsanjose
Tuesday Apr 16, 2013 at 23:03 GMT


Reviewing

@core-ai-bot
Copy link
Member Author

Comment by jasonsanjose
Wednesday Apr 17, 2013 at 00:10 GMT


Initial review complete

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Wednesday Apr 17, 2013 at 18:01 GMT


@jasonsanjose the final UI parts have landed if you'd like to get started reviewing the HTML/LESS while I finish making those changes from your JS feedback.

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Wednesday Apr 17, 2013 at 23:29 GMT


@jasonsanjose fixes pushed. The two issues within the JSON file (uppercase properties and forward slash escaping) are still waiting on Adam, so would you mind spinning them off as a code cleanup bug to unblock the pull request?

@core-ai-bot
Copy link
Member Author

Comment by jasonsanjose
Wednesday Apr 17, 2013 at 23:46 GMT


Sure, see #3471. Reviewing changes.

@core-ai-bot
Copy link
Member Author

Comment by jasonsanjose
Thursday Apr 18, 2013 at 00:14 GMT


Nested scrolling in an inline widget is just as bad as I remembered it. I think it's ok to take for now, but I think it warrants more discussion.

@core-ai-bot
Copy link
Member Author

Comment by jasonsanjose
Thursday Apr 18, 2013 at 03:03 GMT


@peterflynn done with round 2. I think the only thing blocking an immediate merge is the quiet-scrollbars issue. We can file everything else as cleanup. I would have made the change myself and merged, but in just in case, I want to give you the chance to make the call.

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Thursday Apr 18, 2013 at 04:18 GMT


Re the nested scrolling UX: yeah... I think we should do something to improve it a bit before sprint's end. What do you think of the three changes I proposed on email yesterday? (bottom shadow; make scrollbar visible while using arrow keys; don't let scrollwheel/trackpad "overrun" and start scrolling editor).

For me the biggest pain point is the scrollwheel issue... but once the keyboard nav work is in maybe that will become more annoying for many people... hard to say.

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Thursday Apr 18, 2013 at 05:06 GMT


Pushed up another round of fixes

@core-ai-bot
Copy link
Member Author

Comment by jasonsanjose
Thursday Apr 18, 2013 at 16:33 GMT


re: nested scrolling

  • Yes to a below-the-fold indicator
  • Show scrollbars on keyboard nav...mice to have, but lower priority to me than the shadow and the janky nested scroll.
  • Yes to prevent bubbling. Agreed that this is the biggest annoyance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant