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

feat: tooltip on hover #119

Merged
merged 20 commits into from
May 20, 2020
Merged

feat: tooltip on hover #119

merged 20 commits into from
May 20, 2020

Conversation

sapirpol
Copy link
Member

No description provided.

 - @ui5-language-assistant/language-server@1.3.0
 - @ui5-language-assistant/logic-utils@1.1.2
 - @ui5-language-assistant/semantic-model-types@1.2.0
 - @ui5-language-assistant/semantic-model@1.2.0
 - vscode-ui5-language-assistant@1.1.3
 - @ui5-language-assistant/xml-views-completion@1.3.0
 - @ui5-language-assistant/test-utils@1.2.0
@sapirpol sapirpol requested a review from bd82 May 13, 2020 11:45
ast: XMLDocument,
offset: number,
model: UI5SemanticModel
): BaseUI5Node | undefined {
Copy link
Member

Choose a reason for hiding this comment

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

We can probably be more specific than BaseUI5Node and define a "HoverUI5Node` ,e.g:

type HoverUI5Node = UI5Class | UI5Attribute | ....;

return undefined;
}

function findUI5ClassMemberByName(
Copy link
Member

Choose a reason for hiding this comment

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

This should probably go into logic-utils

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree but make it receive the attribute instead of its key

@sapirpol sapirpol requested review from bd82 and tal-sapan May 20, 2020 07:39
Copy link
Contributor

@tal-sapan tal-sapan left a comment

Choose a reason for hiding this comment

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

I didn't go over the tests yet

packages/language-server/src/server.ts Show resolved Hide resolved
packages/xml-views-tooltip/README.md Outdated Show resolved Hide resolved
packages/xml-views-tooltip/api.d.ts Outdated Show resolved Hide resolved
packages/xml-views-tooltip/src/documentation.ts Outdated Show resolved Hide resolved
packages/xml-views-tooltip/package.json Outdated Show resolved Hide resolved
UI5Enum,
BaseUI5Node,
} from "@ui5-language-assistant/semantic-model-types";
import { getNodeDocumentation } from "./documentation";
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be the last import

packages/language-server/src/hover-tooltip.ts Outdated Show resolved Hide resolved
packages/language-server/src/hover-tooltip.ts Outdated Show resolved Hide resolved
packages/xml-views-tooltip/src/tooltip.ts Outdated Show resolved Hide resolved
packages/xml-views-tooltip/src/tooltip.ts Outdated Show resolved Hide resolved
packages/language-server/test/tooltip-spec.ts Outdated Show resolved Hide resolved
packages/language-server/test/tooltip-spec.ts Outdated Show resolved Hide resolved
packages/language-server/test/tooltip-spec.ts Outdated Show resolved Hide resolved
packages/language-server/test/tooltip-spec.ts Outdated Show resolved Hide resolved
packages/language-server/test/tooltip-spec.ts Outdated Show resolved Hide resolved
packages/language-server/test/tooltip-spec.ts Outdated Show resolved Hide resolved
packages/language-server/test/tooltip-spec.ts Outdated Show resolved Hide resolved
packages/language-server/test/tooltip-spec.ts Outdated Show resolved Hide resolved
Copy link
Contributor

@tal-sapan tal-sapan left a comment

Choose a reason for hiding this comment

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

From these comments only the readme comments and the .only comments are important for now

const offset = document.offsetAt(textDocumentPosition.position);
const astPosition = getAstNodeInPosition(ast, offset);
if (astPosition !== undefined) {
const ui5Node = findUI5HoverNodeAtOffset(astPosition, model);
Copy link
Contributor

Choose a reason for hiding this comment

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

Not for now but maybe we should rename findUI5HoverNodeAtOffset because to findUI5HoverNodeAtPosition

packages/language-server/src/hover.ts Outdated Show resolved Hide resolved
packages/xml-views-tooltip/README.md Outdated Show resolved Hide resolved
packages/xml-views-tooltip/README.md Outdated Show resolved Hide resolved
packages/xml-views-tooltip/src/tooltip.ts Outdated Show resolved Hide resolved
packages/xml-views-tooltip/test/tooltip-spec.ts Outdated Show resolved Hide resolved
packages/xml-views-tooltip/test/tooltip-spec.ts Outdated Show resolved Hide resolved
@sapirpol sapirpol requested a review from tal-sapan May 20, 2020 13:51
packages/language-server/package.json Outdated Show resolved Hide resolved
packages/xml-views-tooltip/src/tooltip.ts Show resolved Hide resolved
packages/xml-views-tooltip/src/tooltip.ts Show resolved Hide resolved
packages/xml-views-tooltip/src/tooltip.ts Outdated Show resolved Hide resolved
packages/xml-views-tooltip/test/tooltip-spec.ts Outdated Show resolved Hide resolved
packages/xml-views-tooltip/test/tooltip-spec.ts Outdated Show resolved Hide resolved
packages/xml-views-tooltip/test/tooltip-spec.ts Outdated Show resolved Hide resolved
packages/xml-views-tooltip/test/tooltip-spec.ts Outdated Show resolved Hide resolved
@sapirpol sapirpol requested a review from tal-sapan May 20, 2020 14:44
Copy link
Contributor

@tal-sapan tal-sapan left a comment

Choose a reason for hiding this comment

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

Please fix the readme file. The rest of the comments are minor and can be in a different PR.

packages/xml-views-tooltip/test/tooltip-spec.ts Outdated Show resolved Hide resolved
@sapirpol sapirpol requested a review from tal-sapan May 20, 2020 15:19
@sapirpol sapirpol requested a review from tal-sapan May 20, 2020 15:31
@tal-sapan tal-sapan merged commit e3bf89d into master May 20, 2020
@tal-sapan tal-sapan deleted the hover branch May 20, 2020 15:38
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

Successfully merging this pull request may close these issues.

3 participants