-
Notifications
You must be signed in to change notification settings - Fork 47
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
Chore: Group CSS constants in a more readable manner #150
Conversation
Verified that @pramodsum has signed the CLA. Thanks for the pull request! |
@@ -12,6 +12,8 @@ import { | |||
SELECTOR_ANNOTATION_DIALOG | |||
} from '../../constants'; | |||
|
|||
const SELECTOR_ANNOTATED_ELEMENT = '.annotated-element'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you roll .annotated-element
into constants
. It's been copy-pasted in quite a few places
src/doc/docUtil.js
Outdated
@@ -61,8 +43,10 @@ export function isPresentation(annotatedElement) { | |||
* @return {boolean} Whether or not a dialog is active | |||
*/ | |||
export function hasActiveDialog(annotatedEl) { | |||
const commentsDialogEl = annotatedEl.querySelector(`.${CLASS_ANNOTATION_DIALOG}:not(.bp-is-hidden)`); | |||
const highlightDialogEl = annotatedEl.querySelector(`.${CLASS_ANNOTATION_HIGHLIGHT_DIALOG}:not(.bp-is-hidden)`); | |||
const commentsDialogEl = annotatedEl.querySelector(`.${constants.CLASS_ANNOTATION_DIALOG}:not(.bp-is-hidden)`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you put bp-is-hidden
into constants?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason you import * and name the import vs. import specific classes? {CLASS_FOO} from '/constants'
Initially I think we did that for classes where we're not really using many constants. I do plan on going through and just doing |
No description provided.