Expose handler to allow CSS selectors#1281
Conversation
skjnldsv
left a comment
There was a problem hiding this comment.
I'm getting uncomfortable having text changes merged in viewer thinking
Why can't this be done in Text?
|
@skjnldsv Please check discussions on this PR: |
I should have been mentioned in the other thread 🤷 |
e63d45d to
85b42ca
Compare
src/views/Viewer.vue
Outdated
| size="full" | ||
| :class="{'icon-loading': !currentFile.loaded && !currentFile.failed, | ||
| 'theme--undefined': theme === null, 'theme--dark': theme === 'dark', 'theme--light': theme === 'light', 'theme--default': theme === 'default'}" | ||
| :class="[{'icon-loading': !currentFile.loaded && !currentFile.failed, |
There was a problem hiding this comment.
Make we can move this to a computed property to make it more readable :)
src/views/Viewer.vue
Outdated
| 'theme--undefined': theme === null, 'theme--dark': theme === 'dark', 'theme--light': theme === 'light', 'theme--default': theme === 'default'}" | ||
| :class="[{'icon-loading': !currentFile.loaded && !currentFile.failed, | ||
| 'theme--undefined': theme === null, 'theme--dark': theme === 'dark', | ||
| 'theme--light': theme === 'light', 'theme--default': theme === 'default'}, `app-${handlerId}`]" |
There was a problem hiding this comment.
I'd prefer to set a data attribute like :data-handler="handlerId" as the handler id might also contain characters that are not supported in css selector classes
There was a problem hiding this comment.
I agree, it will be more future-proof and prevents CSS conflicts.
jancborchardt
left a comment
There was a problem hiding this comment.
Looks great design-wise! :)
85b42ca to
e4062b2
Compare
|
/compile amend / |
Signed-off-by: Luka Trovic <luka@nextcloud.com> Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
e4062b2 to
f52dc17
Compare
nextcloud/text#2440