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

Attachment icon is missing #1412

Closed
1 of 2 tasks
georgegevoian opened this issue Jan 31, 2025 · 6 comments · Fixed by #1420
Closed
1 of 2 tasks

Attachment icon is missing #1412

georgegevoian opened this issue Jan 31, 2025 · 6 comments · Fixed by #1420
Labels
bug Something isn't working

Comments

@georgegevoian
Copy link
Contributor

Describe the current behavior

The recent upgrade to Bootstrap 5 appears to have broken the attachment icon in Grist. (See attached screenshot.)

CC @hexaltation

Image

Steps to reproduce

No response

Describe the expected behavior

No response

Where have you encountered this bug?

Instance information (when self-hosting only)

No response

@georgegevoian georgegevoian added the bug Something isn't working label Jan 31, 2025
@hexaltation
Copy link
Collaborator

Hello @georgegevoian,

As it is an atomic commit d036d28
it can be reversed easily.

I can take a look on monday if it's not fixed before.
And add a test to detect that attachment icon properly appears when it is needed.

@georgegevoian
Copy link
Contributor Author

Hello @georgegevoian,

As it is an atomic commit d036d28 it can be reversed easily.

I can take a look on monday if it's not fixed before. And add a test to detect that attachment icon properly appears when it is needed.

Great, thanks @hexaltation!

@hexaltation
Copy link
Collaborator

hexaltation commented Feb 3, 2025

Glyphicon support was removed from Bootstrap since version 4.

It's used in these files :

$ grep -rin glyphicon
client/lib/koForm.js:337:    this.optionButton("left", dom('span.glyphicon.glyphicon-align-left'),
client/lib/koForm.js:339:    this.optionButton("center", dom('span.glyphicon.glyphicon-align-center'),
client/lib/koForm.js:341:    this.optionButton("right", dom('span.glyphicon.glyphicon-align-right'),
client/lib/koForm.js:482:            dom('span.kf_drag_indicator.glyphicon.glyphicon-option-vertical') :
client/lib/koForm.js:487:            return dom('span.drag_delete.glyphicon.glyphicon-remove',
client/components/DocConfigTab.js:19:      dom('span.glyphicon.glyphicon-check'),
client/components/ValidationPanel.js:66:            dom('div.validation_trash.glyphicon.glyphicon-remove',
client/components/ValidationPanel.js:71:            1, dom('div.glyphicon.glyphicon-tag.config_icon'),
client/components/ColumnFilters.css:69:.g-glyphicon-tristate {
client/components/RecordLayoutEditor.js:128:    dom('div.g_record_delete_field.glyphicon.glyphicon-eye-close',
client/widgets/AttachmentsWidget.ts:193:const cssAttachmentIcon = styled('div.glyphicon.glyphicon-paperclip', `

Glyphicon have a pricing model that don't match an open source project (has every self hoster must buy a license).

Octicon seems to be a better solution as it's fee free and under MIT license.

@hexaltation
Copy link
Collaborator

Update
It seems that all icons used in glyphicons have an equivalent in static/icons/icons.css
So I will do a replacement with it rather than add a new dependency in Grist-core

@hexaltation
Copy link
Collaborator

@georgegevoian can you confirm my intuition that this two files are dead code.

client/components/DocConfigTab.js
client/components/ValidationPanel.js

ValidationPanel.js is only referenced in DocConfigTab.js

And DocConfigTab.js is only referenced for a unique autodispose in

this.autoDispose(DocConfigTab.create({gristDoc: this}));


The same for

exports.alignmentSelector = function(valueObservable) {

alignmentsSelector is never called

Do you feel comfortable with the idea that I remove this parts of code.
Or do you think I missed a thing?

@dsagal
Copy link
Member

dsagal commented Feb 4, 2025

OK to remove. There used to be a whole "Validations" feature, but it wasn't polished, so was disabled from UI, and given many years that passed, I am comfortable dropping it entirely.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants